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

PR for beginner issue #17036 #18461

Closed
wants to merge 3 commits into from
Closed

Conversation

Snehaaa24
Copy link

@Snehaaa24 Snehaaa24 commented Jan 14, 2025

Added hints for None when union may not have been narrowed #17036
Please let me know if this issue still needs resolution, as it was originally raised in March 2024.

Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

@sterliakov
Copy link
Collaborator

This doesn't seem related to #17036, are you certain the issue number is correct?

@Snehaaa24
Copy link
Author

Snehaaa24 commented Jan 14, 2025

This doesn't seem related to #17036, are you certain the issue number is correct?

yes, the issue was -Better error message when union may not have been narrowed #17036

@sterliakov
Copy link
Collaborator

Yes, and this PR does nothing related to union/none checks as far as I can tell. One edit is a rephrased comment (actually making the intent less clear, removing the reference to the problem that bit of code solves) and another edit changes a helper script not used by mypy itself (and probably completely dead and unused as it refers to python 3.5 which is long gone), providing no explanation.

@Snehaaa24
Copy link
Author

Yes, and this PR does nothing related to union/none checks as far as I can tell. One edit is a rephrased comment (actually making the intent less clear, removing the reference to the problem that bit of code solves) and another edit changes a helper script not used by mypy itself (and probably completely dead and unused as it refers to python 3.5 which is long gone), providing no explanation.

Alright, sorry for the inconvinience caused. I misunderstood the issue. What I inferred from the issue was that whenever there's a union used add a hint stating that a "none check is required" for beginner users. But as you said, I ended up removing the important comments and the other edit was not needed as well. Thank you for your valuable feedback!

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