-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add pandas-path integration #135
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #135 +/- ##
========================================
- Coverage 93.5% 93.4% -0.2%
========================================
Files 21 22 +1
Lines 1119 1125 +6
========================================
+ Hits 1047 1051 +4
- Misses 72 74 +2
|
@@ -12,6 +12,7 @@ mkdocs-material>=7 | |||
mkdocstrings>=0.15 | |||
mypy | |||
pandas | |||
pandas-path>=0.3.0 |
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.
Wonder if it makes sense to add a pandas
extras option.
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 kind of conceptually nice to have the extras be focused on cloud providers since that's the major differentiator.
pip install pandas-path | ||
``` | ||
|
||
All you need to do to register the accessor is `from cloudpathlib.pandas import cloud`. |
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.
Why the extra module layer, instead of just import cloudpathlib.pandas
?
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.
This is to mirror the the pandas-path import which looks like from pandas_path import path
. I think it's nice to (1) make the structure similar, and (2) see the name of the accessor when you do the import so you can see how they might get tied together.
@jayqi Let me know if you have thoughts on putting this functionality in cloudpathlib. Now that it is implemented, I could also see this making sense in the |
Yeah, I guess it could go either way:
Now that I've seen it, I guess the interface looks cleaner to have this live in |
@pjbull the more I've thought about this one, the more that I think having this as an extension within |
.cloud
accessor topd.Series
andpd.Index
via a pandas-path custom accessor