-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Lessen dmypy suggest path limitations for Windows machines #19337
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
Conversation
I wonder if it would make sense to just remove the |
That could certainly be an option, or make it always max of 2 if a platform check is unacceptable. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG! Added a comment suggestion (it wouldn't be immediately obvious to me why a second colon should be allowed on windows) - feel free to ignore if you think it's me too used to convenient devenv and any normal person should immediately understand that:)
Co-authored-by: Stanislav Terliakov <50529348+sterliakov@users.noreply.github.com>
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you!
In this pull request, we allow dmypy suggest absolute paths to contain the drive letter colon for Windows machines. Fixes #19335. This is done by changing how `find_node` works slightly, allowing there to be at most two colon (`:`) characters in the passed key for windows machines instead of just one like on all other platforms, and then using `rsplit` and a split limit of 1 instead of just `split` like prior. --------- Co-authored-by: Stanislav Terliakov <50529348+sterliakov@users.noreply.github.com>
In this pull request, we allow dmypy suggest absolute paths to contain the drive letter colon for Windows machines. Fixes #19335.
This is done by changing how
find_node
works slightly, allowing there to be at most two colon (:
) characters in the passed key for windows machines instead of just one like on all other platforms, and then usingrsplit
and a split limit of 1 instead of justsplit
like prior.I was looking at the existing tests for dmypy suggest and noticed nothing is testing absolute paths for any platform, so I am unsure what I need to do to add tests; This sounds like maybe it could require changes to the test framework system, and I'd like feedback from others before trying to do that.