Skip to content
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

Add annotations and py.typed to conform with PEP561 #31

Closed
wants to merge 8 commits into from

Conversation

keiclone
Copy link
Contributor

@keiclone keiclone commented May 4, 2020

*Issue number of the reported bug or feature request: #24

Describe your changes
made attrs-strict PEP561 compatible by adding annotations and py.typed. This removes the missing import error from mypy when attempting to run static type checking on packages using attrs-strict

Testing performed
Was able to successfully run static type checking in a test project after installing attrs-strict from my branch.

when testing on a separate library:

# test.py
@attr.s
class FooClass(object):
    list_of_numbers = attr.ib(validator=type_validator(), type=List[int])


fc = FooClass([1, 2, 3, "four"])
# before changes
$ pipenv install attrs-strict
Installing git+https://github.com/bloomberg/attrs-strict
...
$ mypy test.py
error: Cannot find implementation or library stub for module named 'attrs_strict'
note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports
error: List item 3 has incompatible type "str"; expected "int"

# after changes
$ pipenv install git+https://github.com/keiclone/attrs-strict#egg=attrs-strict
Installing git+https://github.com/keiclone/attrs-strict#egg=attrs-strict
...
$ mypy test.py
error: List item 3 has incompatible type "str"; expected "int"

Additional context
could use some extra attention on how this was added to tox to make sure it was done correctly

future changes to attrs_strict will need to ensure the stubs are updated appropriately as this will not come for free as long as we need to support python 2

@keiclone keiclone changed the title add annotations and py.typed (take 2)less add annotations and py.typed (take 2) May 4, 2020
@erikseulean
Copy link
Contributor

This is great, thanks for the hard work. Could you please squash your commits ?

@gaborbernat since you have the most experience on this, I'd like to hear your comments on this.

@erikseulean
Copy link
Contributor

Could you please capitalise the commit and add a description of what it does ? Helps with the docs...

@keiclone keiclone changed the title add annotations and py.typed (take 2) Add annotations and py.typed (take 2) May 8, 2020
@keiclone keiclone changed the title Add annotations and py.typed (take 2) Add annotations and py.typed to conform with PEP561 May 8, 2020
@keiclone
Copy link
Contributor Author

keiclone commented May 8, 2020

@erikseulean made a few changes. lemme know if they work?

setup.cfg Outdated
@@ -1,2 +1,12 @@
[bdist_wheel]
universal = true

[mypy]
python_version = 3.7
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be not set here as we need to run against both python 2 and 3, given we support both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can remove this. we'd have to set two different runs in tox then, one for each version

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, can do that in tox.ini as separate commands one after another

setup.cfg Outdated Show resolved Hide resolved
@@ -0,0 +1,2 @@
from ._error import AttributeTypeError as AttributeTypeError, BadTypeError as BadTypeError, TupleError as TupleError, TypeValidationError as TypeValidationError, UnionError as UnionError
Copy link
Contributor

Choose a reason for hiding this comment

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

we need some tool that validates that content from the python file matches with the stubs; for example https://pypi.org/project/retype/ is an example, please use it in our tox check... see https://github.com/ambv/retype/blob/master/tox.ini#L58

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wasn't aware of this. will add!

setup.py Outdated Show resolved Hide resolved
@erikseulean
Copy link
Contributor

This will need rebase.

@keiclone keiclone marked this pull request as draft May 26, 2020 03:42
@keiclone keiclone force-pushed the master branch 2 times, most recently from 39b3bd3 to 372a361 Compare May 26, 2020 07:17
* Black format

Signed-off-by: Erik-Cristian Seulean <[email protected]>

* Fix handling of typing.Any

So far Any was not taken in consideration and it was failing
to handle it correctly. If the base_type is Any, stop the
recursive check and return as if everything is fine.

Signed-off-by: Erik-Cristian Seulean <[email protected]>
Signed-off-by: Wilfred Wong <[email protected]>
This would enable custom made types that extend
collections.abc.Mapping and collections.abc.MutableMapping

Signed-off-by: Erik-Cristian Seulean <[email protected]>
Signed-off-by: Wilfred Wong <[email protected]>
saytosid and others added 6 commits May 26, 2020 03:21
typing.NewType is actual just a function and should be interpreted as
an alias for something. However its __str__
‘NewType.<locals>.new_type’, so it includes neither its custom name nor
the type for which it is an alias, which is not very helpful to
identify why a certain type check failed. Hence, such information
are also printed in error messages, but other types are left as they
were before.

Signed-off-by: Fabian Raab <[email protected]>
Signed-off-by: Wilfred Wong <[email protected]>
Fixes: bloomberg#35

Signed-off-by: Siddhant Kumar <[email protected]>
Signed-off-by: Wilfred Wong <[email protected]>
Signed-off-by: Siddhant Kumar <[email protected]>
Signed-off-by: Wilfred Wong <[email protected]>
Signed-off-by: Wilfred Wong <[email protected]>

mypy --strict. add MANIFEST.in. run tox on both python versions

Signed-off-by: Wilfred Wong <[email protected]>

rework type annotations as necessary

Signed-off-by: Wilfred Wong <[email protected]>
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.

5 participants