Skip to content

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

Closed
filbranden wants to merge 1 commit intopython-attrs:mainfrom
filbranden:convertertype1
Closed

Fix type signature for callables passed to Converter with takes_self=True#1382
filbranden wants to merge 1 commit intopython-attrs:mainfrom
filbranden:convertertype1

Conversation

@filbranden
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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