Skip to content
New issue

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

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

Already on GitHub? # to your account

Advertise typing support #10

Merged
merged 2 commits into from
Mar 1, 2024
Merged

Advertise typing support #10

merged 2 commits into from
Mar 1, 2024

Conversation

alexfikl
Copy link
Contributor

The main point of this was to add a py.typed marker so that users can get the typing annotations.

To make sure that all works nicely, it also

  • Adds a barebones mypy configuration
  • Adds a mypy step to the CI
  • Fixes all the reported issues by mypy --strict.

Copy link
Owner

@pierre-24 pierre-24 left a comment

Choose a reason for hiding this comment

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

Thank you, I did not know that it existed. There are small things to correct (thanks for correcting the lint error, btw), but we're almost good to go :)

@@ -46,17 +46,17 @@ def __repr__(self) -> str:


class Lexer:
def __init__(self, inp, stopwords: List[str]):
def __init__(self, inp: str, stopwords: List[str]) -> None:
Copy link
Owner

Choose a reason for hiding this comment

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

That should be "Lexer" not None, then :)

Copy link
Contributor Author

@alexfikl alexfikl Mar 1, 2024

Choose a reason for hiding this comment

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

__init__ always returns None (__new__ returns the type of the object)
https://docs.python.org/3/reference/datamodel.html#object.__init__

Copy link
Owner

Choose a reason for hiding this comment

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

Oh, ok, I thought they were returning the Object itself. Good to know :)

@@ -1,43 +1,54 @@
[build-system]
build-backend = "setuptools.build_meta"
Copy link
Owner

Choose a reason for hiding this comment

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

Is this mandatory? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand correctly, setuptools is the default at the moment
https://peps.python.org/pep-0518/#build-system-table
But I guess it's good to be explicit?

Copy link
Owner

Choose a reason for hiding this comment

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

OK, then :)

@alexfikl
Copy link
Contributor Author

alexfikl commented Mar 1, 2024

Thank you, I did not know that it existed. There are small things to correct (thanks for correcting the lint error, btw), but we're almost good to go :)

Thanks for the very quick reply! Yeah, adding a py.typed empty file is required to let other packages know they can use the typing information:
https://peps.python.org/pep-0561/#packaging-type-information

@pierre-24 pierre-24 merged commit 8cc47b5 into pierre-24:dev Mar 1, 2024
5 checks passed
@pierre-24
Copy link
Owner

Thank you :)

@alexfikl alexfikl deleted the add-typing branch March 1, 2024 08:57
@alexfikl
Copy link
Contributor Author

alexfikl commented Mar 1, 2024

Thank you for the quick review! 😁

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

Successfully merging this pull request may close these issues.

2 participants