-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
QDataFrame #45602
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
QDataFrame #45602
Conversation
Hello @wkerzendorf! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:
|
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 cannot be pandas code proper. we don't depend on astropy (and won't).
however would not object to this in a test.
I really advise you guys to implement an ExtensionArray rather than a DataFrame/Series subclass. See #45544 |
I'd be leery to even include this as a test in pandas. I think we don't really have a prescient of testing 3rd party package EA in our CI and I think better left tested by astropy. |
I guess a try:
import astropy
except ImportError:
raise ImportError('QDataFrame requires astropy') is not an acceptable solution to you? (I know you do this with pytables and HDF5). In any case, the community really wants such a feature - am happy to implement this and host this elsewhere. I would need some help with |
That import pattern is typically used for internal IO support. I would recommend this new ExtenstionArray living in astropy or an astropy project repo like other projects have done: https://pandas.pydata.org/docs/ecosystem.html?highlight=ecosystem#extension-data-types You can reference https://pandas.pydata.org/docs/development/extending.html for a guide for developing the ExtensionArray |
closing. we would take an EA implementation as a test (but likey better off done as an external package). |
Not sure where that fits inside pandas. Happy to add tests to this but this works AFAICT.