Skip to content

Fix crash in attrs_plugin when using a converter #4583

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

Closed
wants to merge 1 commit into from

Conversation

euresti
Copy link
Contributor

@euresti euresti commented Feb 17, 2018

Found this issue running mypy on our codebase.

Sadly I don't know how to fix the problem completely and support subclasses and incremental mode but I'll leave that for another day.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

It's definitely better than nothing. :-)

glyph added a commit to glyph/klein that referenced this pull request Feb 20, 2018
@euresti
Copy link
Contributor Author

euresti commented Feb 20, 2018

Any idea when the release might be? I have a better fix that makes it work correctly for local functions. Just wanting to know if I should rush on that.

@euresti
Copy link
Contributor Author

euresti commented Feb 20, 2018

My fix builds on this change so this should land too.

@gvanrossum
Copy link
Member

Here's the release schedule: #4606

@euresti
Copy link
Contributor Author

euresti commented Feb 23, 2018

Can this be merged? I'd hate for someone who's testing out the attrs support in master getting crashes. Or the better fix in #4607 but that needs review.

@ilevkivskyi
Copy link
Member

So if #4607 is merged, then this is unnecessary, right? In this case I would wait for whenever #4607 lands (I will review it now) rather than applying a temporary fix.

@euresti euresti deleted the fix_attrs_converter_crash branch February 23, 2018 20:06
yedpodtrzitko pushed a commit to kiwicom/mypy that referenced this pull request Mar 15, 2018
Fixes python#4583 (and more).

My first pass at this only supported converters that were reachable
from the root.  With this code the converter can be a local function
too.

Also now when we can't detect the type of the converter the init type
becomes Any.
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