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 attr.__version_info__ #580

Merged
merged 10 commits into from Oct 1, 2019
Merged

Add attr.__version_info__ #580

merged 10 commits into from Oct 1, 2019

Conversation

@hynek
Copy link
Member

@hynek hynek commented Sep 25, 2019

This allows users to check for features and avoid deprecation warnings without
breaking backward compatibility.

This is especially important with the cmp → order/eq split.


@euresti do I need to add it to __init__.pyi if it's an attrs class? I tried to just import it into it, but that started throwing errors about stuff in _make.py

@hynek hynek added this to the 19.2 milestone Sep 25, 2019
docs/api.rst Outdated Show resolved Hide resolved
@euresti
Copy link
Contributor

@euresti euresti commented Sep 25, 2019

I think you should add it to the init.pyi and the other missing ones like this:

__version__: str
__title__: str
__description__: str
__url__: str
__uri__: str
__author__: str
__email__: str
__license__: str
__copyright__: str

@hynek
Copy link
Member Author

@hynek hynek commented Sep 25, 2019

Technically, mypy knows the types of VersionInfo…but if I import it, I get the errors…is there a way around here? Or do I have to either fix mypy for attrs or duplicate the information?

src/attr/_compat.py:18: error: Cannot assign multiple types to name "ordered_dict" without an explicit "Type[...]" annotation
src/attr/_make.py:1538: error: All conditional function variants must have identical signatures
src/attr/_make.py:1907: error: Cannot assign to a type
src/attr/_make.py:1909: error: "Attribute" has no attribute "hash"
src/attr/_make.py:2039: error: Cannot assign to a type

are the new ones.

@euresti
Copy link
Contributor

@euresti euresti commented Sep 25, 2019

Oh. You shouldn't run mypy on attrs source code. At least we haven't made it pass type checking. The only file that passes type checking is typing_example.py

@euresti
Copy link
Contributor

@euresti euresti commented Sep 25, 2019

Although:

src/attr/_make.py:1538: error: All conditional function variants must have identical signatures

is complaining that

            def fmt_setter(attr_name, value_var):
            def fmt_setter(attr_name, value_var):
        # Not frozen.
        def fmt_setter(attr_name, value):

the variable name changed in the 3rd instance

@hynek
Copy link
Member Author

@hynek hynek commented Sep 26, 2019

@euresti
Copy link
Contributor

@euresti euresti commented Sep 26, 2019

Oh I see. You added _version.py you'll probably have to add _version.pyi
But I can't repro your error messages. What command are you running to trigger them?

hynek added 4 commits Sep 26, 2019
This allows users to check for features and avoid deprecation warnings without
breaking backward compatibility.
@hynek
Copy link
Member Author

@hynek hynek commented Sep 26, 2019

Now that I've rebased, if I add a line from ._version import VersionInfo to __init__.pyi without adding a _version.pyi, I get:

❯ tox -e typing
typing inst-nodeps: /Users/hynek/Projects/attrs/.tox/.tmp/package/1/attrs-19.2.0.dev0.tar.gz
typing installed: atomicwrites==1.3.0,attrs==19.2.0.dev0,coverage==4.5.4,hypothesis==4.36.0,importlib-metadata==0.22,more-itertools==7.2.0,mypy==0.720,mypy-extensions==0.4.1,packaging==19.1,pluggy==0.13.0,py==1.8.0,Pympler==0.7,pyparsing==2.4.2,pytest==5.1.2,six==1.12.0,typed-ast==1.4.0,typing-extensions==3.7.4,wcwidth==0.1.7,zipp==0.6.0,zope.interface==4.6.0
typing run-test-pre: PYTHONHASHSEED='3295264721'
typing run-test: commands[0] | mypy src/attr/__init__.pyi src/attr/converters.pyi src/attr/exceptions.pyi src/attr/filters.pyi src/attr/validators.pyi
src/attr/_compat.py:18: error: Cannot assign multiple types to name "ordered_dict" without an explicit "Type[...]" annotation
src/attr/_make.py:1538: error: All conditional function variants must have identical signatures
src/attr/_make.py:1907: error: Cannot assign to a type
src/attr/_make.py:1909: error: "Attribute" has no attribute "hash"
src/attr/_make.py:2039: error: Cannot assign to a type
ERROR: InvocationError for command /Users/hynek/Projects/attrs/.tox/typing/bin/mypy src/attr/__init__.pyi src/attr/converters.pyi src/attr/exceptions.pyi src/attr/filters.pyi src/attr/validators.pyi (exited with code 1)

You added _version.py you'll probably have to add _version.pyi

  1. Let's appreciate the irony of me saying that we don't add files a lot yesterday.
  2. So I'll have to replicate the type information I guess? :(

@hynek
Copy link
Member Author

@hynek hynek commented Sep 26, 2019

OK I'm not happy but I think this it r4r now. And I thought this is gonna a bunch of lines – I'll never learn! 🤪

src/attr/__init__.pyi Show resolved Hide resolved
src/attr/_version.pyi Outdated Show resolved Hide resolved
@euresti
Copy link
Contributor

@euresti euresti commented Sep 26, 2019

All the typing stuff looks good to me!

@hynek
Copy link
Member Author

@hynek hynek commented Sep 26, 2019

Thanks David!

Anyone else for the rest? :)

@hynek hynek mentioned this pull request Oct 1, 2019
euresti
euresti approved these changes Oct 1, 2019
Copy link
Contributor

@euresti euresti left a comment

I don't exactly know the protocol but I only made 2 comments which you can fix before merging if you want.

docs/api.rst Outdated Show resolved Hide resolved
tests/test_version.py Outdated Show resolved Hide resolved
@hynek
Copy link
Member Author

@hynek hynek commented Oct 1, 2019

Thanks, you followed the protocol perfectly! 💛

@hynek hynek merged commit 955d622 into master Oct 1, 2019
18 checks passed
@hynek hynek deleted the version-info branch Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants