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

Fix type signature for callables passed to Converter with takes_self=True #1382

Closed
wants to merge 1 commit into from

Conversation

filbranden
Copy link
Contributor

@filbranden filbranden commented Dec 13, 2024

Summary

Fix type signature for callables passed to Converter with takes_self=True, so that using the specific attrs class that contains the field with the converter is valid in the callable type signature.

Fixes #1378.

Note that python/mypy#15736 still triggers mypy errors when passing a Converter instance as a converter= kwarg to a field, so work around that using # type: ignore[misc] in test cases.

Pull Request Check List

  • Do not open pull requests from your main branch – use a separate branch!
  • Added tests for changed code.
  • N/A 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.
    • N/A If they've been added to attr/__init__.pyi, they've also been re-imported in attrs/__init__.pyi.
  • N/A Updated documentation for changed code.
  • N/A Documentation in .rst and .md files is written using semantic newlines.
  • N/A Changes (and possible deprecations) have news fragments in changelog.d.
  • Consider granting push permissions to the PR branch, so maintainers can fix minor issues themselves without pestering you.

@Tinche
Copy link
Member

Tinche commented Dec 13, 2024

So what exact issue are we trying to solve here?

There's kind of no point in using a TypeVar if we're going to reference it only once - the usual use case is to link a type in the function parameters to a type in the function return value, or a type in the class to a type in a method.

@filbranden
Copy link
Contributor Author

@Tinche see #1372, though the author looped back in and mentioned finding a workaround.

But I think it's not unreasonable that a converter for a field in attrs class C would take an instance of class C as its argument.

Indeed it would be better if we could link the TypeVar to the definition, but I'm not sure how to do that part, because that would involve taking the class in which a field(converter=...) was called.

If you think the workaround in #1372 (taking an attrs.AttrsInstance) is fine, then it should be good to just drop this, though we might consider improving documentation to indicate that's the case. Thanks!

@filbranden
Copy link
Contributor Author

Closing, as per discussion using attrs.AttrsInstance is correct and acceptable.

@filbranden filbranden closed this Dec 24, 2024
@filbranden filbranden deleted the convertertype1 branch December 24, 2024 02:56
@hynek
Copy link
Member

hynek commented Dec 24, 2024

Thanks for the discussion tho!

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.

Cannot get attrs.Converter() with takes_self pass through mypy
3 participants