Skip to content

Add typing throughout code base #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add typing throughout code base #12

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
33cf6d2
fix: added input/output dirs to .gitignore
jhaco Dec 2, 2023
7bf320f
doc: added type-hints
jhaco Dec 2, 2023
6351f89
doc: more precise hints for dict; use OR instead of | for python <3.10
jhaco Dec 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions smdatatools/data_processing/input_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class InputProcessor:
def convert_note(line: str) -> str:
return sub('4', '1', sub('[MKLF]', '0', line)) #replaces extra notes: M, K, L, F; replaces 4 note

def parse_sm_input(sm_file: list[str]) -> tuple[defaultdict(list), bool]:
def parse_sm_input(sm_file: list[str]) -> tuple[dict[str, dict[str or list[str]] or float or int ], bool]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check my commit message. For Python version <3.10, I have to use OR instead of a Union operator. If you can test it from your end with a Python version 3.10+ and it still works then there shouldn't be any problem.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then Union[x, y, z] from typing should be used instead. As is, I don't think this PR passes pytype checks.

IMO the code base can be upgraded to Python 3.10+. 3.10 has been out for 2 years and it's worth using more modern features.

note_data = defaultdict(list)
note_data['notes'] = defaultdict(list) # notes are paired with each difficulty
current_difficulty = ''
Expand Down Expand Up @@ -75,7 +75,7 @@ def parse_sm_input(sm_file: list[str]) -> tuple[defaultdict(list), bool]:

return note_data, valid

def parse_txt_input(txt_file: list[str]) -> defaultdict(list):
def parse_txt_input(txt_file: list[str]) -> dict[str, dict[str or list[str]] or float or int ]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typing uses the Union operator |

https://peps.python.org/pep-0604/

note_data = defaultdict(list)
note_data['notes'] = defaultdict(list)
current_difficulty = ''
Expand Down