-
Notifications
You must be signed in to change notification settings - Fork 6
fix importing and exporting Session objects from/to CSV and Excel #802
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
Conversation
ba8e2fe
to
2829ad5
Compare
- included key associated with Axis and Group objects when exporting Session objects to CSV or Excel files - removed useless kwargs['name'] = key in PandasHDFHandler._read_item()
2829ad5
to
1e83393
Compare
…xtracting Axis and Group objects from __axes__ and __groups__ special sheets/csv files
…n using Session.save() and Session.load()
…mes property at the beginning to to make LArray.to_frame() consistent with LArray.dump() (and then to make it possible to handle anonymous and/or wildcard axes when dealing with CSV and Excel formats)
…presents an anonymous and/or wildcard axis
1e83393
to
830388c
Compare
@@ -1185,22 +1185,21 @@ def to_frame(self, fold_last_axis_name=False, dropna=None): | |||
b1 6 7 | |||
""" | |||
columns = pd.Index(self.axes[-1].labels) | |||
axes_names = self.axes.display_names[:] |
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.
Are you sure using display_names here is a good idea? You'll get *
in axes names for wildcard axes and {axis_pos}
for anonymous axes...
I am very nervous about this change. I fear it must break "something" somewhere given that to_frame is used in a lot of places (at least indirectly). Pandas dataframes with "{0}*" as explicit name would be ugly, right?
As will be plots with such labels.
* fixed NaNs and None labels appearing in axes and groups when reading/exporting sessions | ||
from/to CSV and Excel files (closes :issue:`804`). | ||
|
||
* fixed importing/exporting anonymous and/or wildcard axes to CSV and Excel files |
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.
Was the problem present only when importing/exporting the axes objects themselves or arrays with such axes, or both?
@@ -55,4 +55,11 @@ Miscellaneous improvements | |||
Fixes | |||
^^^^^ | |||
|
|||
* fixed something (closes :issue:`1`). | |||
* fixed reading/exporting sessions containing two or more axes/groups |
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.
The work you have put into this PR makes me wonder even more whether saving sessions (and especially when using non LArray objects) to CSV and to Excel, are worth it. I fear they will give us an endless stream of problems for little benefit.
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.
No one noticed the bug meaning no one will notice if we drop the ability to save and load axes and groups with the CSV or Excel format.
So, do I edit the title of the corresponding issue and drop Axis and Group objects when calling Session.save()/load() ?
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 am unsure where to draw the line/what's best:
- support saving axes and groups (and possibly arrays with metadata) only to HDF (or other formats where we can store arbitrary things). In that case, we will need to output a warning when saving a session containing them to csv/excel. Or we could go the GIMP route and say there is a single "native" format (HDF or .la or whatever) and there are other formats we can export to, each with their own limitations.
- only support saving sessions to HDF-like formats.
- keep trying to support everything for all formats. Let's be clear: it would love to have that, but I fear the time spent on that could be better spent elsewhere.
- possibly another intermediate option I am not seeing?
I think we need to discuss this face to face, as it will be easier.
arr = ndtest((Axis(2), Axis(2), Axis(2))) | ||
df = arr.to_frame() | ||
assert df.index.name is None | ||
assert df.index.names == ['{0}*', '{1}*'] |
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.
is it really what we want???
closes this PR because of issue #815 and comment #802 (comment) |
No description provided.