Repository navigation
Lessen dmypy suggest path limitations for Windows machines - #19337
hauntsaninja merged 2 commits into
Conversation
Sorry, something went wrong.
|
I wonder if it would make sense to just remove the key.count(":") check |
Sorry, something went wrong.
|
That could certainly be an option, or make it always max of 2 if a platform check is unacceptable. |
Sorry, something went wrong.
This comment has been minimized.
sterliakov
left a comment
There was a problem hiding this comment.
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:)
Sorry, something went wrong.
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. ✅ |
Sorry, something went wrong.
hauntsaninja
left a comment
There was a problem hiding this comment.
Looks good, thank you!
Sorry, something went wrong.
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 using rsplit and a split limit of 1 instead of just split 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.