-
Notifications
You must be signed in to change notification settings - Fork 366
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
MNT: Use pathlib throughout the codebase #2116
Conversation
@@ -24,6 +24,7 @@ | |||
sys.exit(error) | |||
|
|||
|
|||
from pathlib import Path |
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.
E402 module level import not at top of file
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.
Do we want to drop the Python 2 crutch and allow the imports to be at the top?
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.
I think it's be nice to remove, but maybe we should wait until we have wheels? Then it's generally a given if you're building from source, you need to be certain everything's good.
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.
Yes, that seems reasonable. I'm not sure when we'll get wheels here... Still a few PRs needed even if/when we remove GEOS.
c1b9ae4
to
4e39b7f
Compare
40e3cdc
to
db4e536
Compare
db4e536
to
f1c9bd2
Compare
e4d1ab0
to
eee02fc
Compare
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.
Other than CircleCI in #2140.
Change os.path usages to pathlib.Path()
eee02fc
to
707961b
Compare
Change
os.path
usages topathlib.Path()