-
Notifications
You must be signed in to change notification settings - Fork 34
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
Construct frame with columns/index but no data #71
Construct frame with columns/index but no data #71
Conversation
Codecov Report
@@ Coverage Diff @@
## master #71 +/- ##
==========================================
+ Coverage 94.71% 94.79% +0.08%
==========================================
Files 18 18
Lines 5354 5361 +7
==========================================
+ Hits 5071 5082 +11
+ Misses 283 279 -4
Continue to review full report at Codecov.
|
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.
Thanks for taking on this tricky one! It looks like this PR breaks a couple of things, though, so it may need some more work:
Thanks, @colinalexander for this implementation. As @brandtbucher points out, supporting generators as index and column constructors is important. But I have a few additional considerations. If we have a 2D array with one axis of size 0 ("non-fillable"), we cannot store any data, but our array will still have a type. Given that observation: The meaning of The default of This leaves the question of what type the non-fillable array should be. Here, I think we should follow NumPy, which defaults to float64 in similar scenarios. This, I do not think we should use In [8]: np.array(())
Out[8]: array([], dtype=float64)
In [9]: np.empty((0,3))
Out[9]: array([], shape=(0, 3), dtype=float64) |
Notice also that even the current support for a non-fillable shape (with an index but no columns) is flawed in that it returns a In [18]: f = sf.Frame(None, index=(1,2,3), columns=None)
In [20]: f._blocks.shape
Out[20]: (0, 0)
In [22]: f.shape
Out[22]: (0, 0)
In [23]: f.values
Out[23]: array([], shape=(0, 0), dtype=float64) |
0e4bae5
to
8c18c4e
Compare
refactor refactor refactor
8c18c4e
to
6bbade1
Compare
@flexatone and @brandtbucher Thanks for your detailed comments and patience. I believe this revised PR achieves most of your objectives without the need to enhance Regarding the data type, I have filled missing data with
Some examples per the new functionality:
I have added some tests per I have also modified existing behavior. Currently, a frame that drops all of its columns results in a Frame with shape |
Hi @colinalexander. It turns out that a nice solution for this was available by expanding Then, in Apologies for implementing an alternative while you were working on this PR: the approach in Please see my changes here: It would be great if you can recast your PR to include the new tests you have authored. I see also that my solution has not addressed the good issue that you raise: when extracting a In general, I am not sure that putting an unfillable array in |
Thanks Chris. What did you want to use as the datatype for
I'll just close this PR and create a new one with the additional tests from Frame construction using an iterable. |
Per issue #65, this PR allows construction of frames with no data but columns, an index or both.