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

Narrower typing on Factory called with takes_self #780

Merged
merged 4 commits into from
Mar 24, 2021
Merged

Narrower typing on Factory called with takes_self #780

merged 4 commits into from
Mar 24, 2021

Conversation

dargueta
Copy link
Contributor

@dargueta dargueta commented Mar 11, 2021

This changes the annotations for attr.Factory so that the acceptable type of the callable changes based on takes_self.
Theoretically, MyPy will let you do this:

attr.ib(default=attr.Factory(list, takes_self=True)
attr.ib(default=attr.Factory((lambda self: []), takes_self=False)

The change in annotations will prevent that from happening.

As written I think these annotations would require the type checker to be running on Python 3.8 because of the use of Literal. There are a couple ways around this, if necessary.

Pull Request Check List

  • Added tests for changed code.
  • New features have been added to our Hypothesis testing strategy.
  • Changes or additions to public APIs are reflected in our type stubs (files ending in .pyi).
    • ...and used in the stub test file tests/typing_example.py.
  • Updated documentation for changed code.
    • New functions/classes have to be added to docs/api.rst by hand.
    • Changes to the signature of @attr.s() have to be added by hand too.
    • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives. Find the appropriate next version in our __init__.py file.
  • Documentation in .rst files is written using semantic newlines.
  • Changes (and possible deprecations) have news fragments in changelog.d.

@dargueta
Copy link
Contributor Author

dargueta commented Mar 11, 2021

Looks like it's blowing up because of python/typed_ast#156 which was only fixed four days ago in python/typed_ast#155. There's been no release yet, so for the moment MyPy is uninstallable on CPython 3.10.0a6 and later. I think using 3.10.0a5 or lower might fix the issue?

Copy link
Contributor

@euresti euresti left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Lovely to see more people get involved in the type annotations.

I think the problem won't be which python you're running but which python you're type-checking for.

For example:

[mypy]
python_version=3.7

or --python-version 3.7

What appears to happen in that case is that Literal doesn't exist and the type of takes_self gets an Any. This sees happens silently by the way (at least when I ran it I got no error messages). I left comments on how to fix this.

I wonder if we could convince mypy to allow using Literal in stub files even if they aren't present in the current python. Literal is just so useful.

src/attr/__init__.pyi Outdated Show resolved Hide resolved
@dargueta dargueta requested a review from euresti March 22, 2021 18:07
@hynek
Copy link
Member

hynek commented Mar 23, 2021

Pre-commit’s isort is failing for __init__.pyi.

@hynek hynek merged commit 51e6dcf into python-attrs:main Mar 24, 2021
@hynek
Copy link
Member

hynek commented Mar 24, 2021

Thanks!

@dargueta dargueta deleted the factory-self-annotation branch March 24, 2021 22:38
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.

3 participants