Skip to content

Makes function signatures as explicit as possible #197

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.

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 1 commit into
base: master
Choose a base branch
from

Conversation

chaburkland
Copy link
Collaborator

No description provided.

@chaburkland chaburkland requested a review from flexatone August 11, 2025 20:36
@chaburkland chaburkland self-assigned this Aug 11, 2025
Copy link
Contributor

@flexatone flexatone left a comment

Choose a reason for hiding this comment

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

Overall, these are great changes. I had not done this yet, as we were still testing on some old versions of Numpy did not support generic specification. I think, however, we should not have any issues... assuming we can get CI to pass!

@@ -119,7 +129,7 @@ class FrozenAutoMap:
def __init__(self, labels: tp.Iterable[_TLabel] | np.ndarray = (), /,) -> None: ...
def get(self, __key: _TLabel, /,) -> int: ...
def keys(self) -> tp.Iterator[_TLabel]: ...
def items(self) -> tp.Iterator[tuple[_TLabel, int]]: ...
def items(self) -> tp.Iterator[tp.Tuple[_TLabel, int]]: ...
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use tuple[_TLabel, int]] here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was for consistency in the file! Everywhere else used tp.Tuple so I changed it here too

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, well, lets update them all to use tuple; tp.Tuple is deprecated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants