Skip to content
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 ability to include serialized data in glu file #661

Merged
merged 4 commits into from
Jun 19, 2015

Conversation

astrofrog
Copy link
Member

When saving a file, the user can select to create a tar file including a glue file and associated data files.

When the user wants to then access the data in this file, they have to expand the file and then open the glu file inside the directory.

I should probably change to using zip files. Also, at the moment there is no way to load the tar file directly from glue (but we could try and make it that everything is loaded directly from the tar file).

@astrofrog astrofrog changed the title Add ability to save tar bundle Add ability to save zip bundle Jun 6, 2015
@ChrisBeaumont
Copy link
Member

I think the details of "packing" into an archive should be as transparent as possible -- Glue should be able to re-load a packed file directly, and maybe it should even still use the .glu extension (or .glp).

@astrofrog
Copy link
Member Author

I agree about making it seamless, I will work on this next.

@ChrisBeaumont
Copy link
Member

Also I just saw in Twitter that python's zip module holds a copy of the
full file in memory under some circumstances. We should make sure we aren't
doing that here.

On Sunday, June 7, 2015, Thomas Robitaille notifications@github.com wrote:

I agree about making it seamless, I will work on this next.


Reply to this email directly or view it on GitHub
#661 (comment).

@astrofrog
Copy link
Member Author

@ChrisBeaumont - ok, thanks for the heads-up. I'll proceed assuming this is not an issue and we can do some benchmarking once it's implemented.

@astrofrog
Copy link
Member Author

I'm going to work on this more for a bit now.

@astrofrog
Copy link
Member Author

So I guess one question is whether we need to preserve the original file type. Since not all loaders are guaranteed to be able to deal with file handles instead of filenames, it would be much simpler to simply have a single way to serialize Data objects and would save hassle when re-loading them. This is especially the case if we consider that in future, we might have a helper to have the user interactively say how to load an ASCII file. If we save the ASCII file to the archive, the user trying to open the session might have to do the same again.

TL;DR: I think we should serialize Data objects to the zip archive instead of copying the original data file. @ChrisBeaumont - do you agree?

@ChrisBeaumont
Copy link
Member

Would you still support the "lightweight" format that uses file references instead of copies of the data? I'd prefer that that behavior be preserved.

But for the packed version, I don't see the harm in serializing the data instead of the original file contents.

@astrofrog
Copy link
Member Author

@ChrisBeaumont - yes, we could support both, and the default could remain the current format. So basically it would be exactly the same as now but with the optional ability to serialize data (and we can keep the .glu extension for both).

@ChrisBeaumont
Copy link
Member

That sounds like a good plan to me

@astrofrog
Copy link
Member Author

@ChrisBeaumont - I started from scratch and it turned out to be very easy to do this! At the moment this does not save the data in compressed form, though that would be easy to do by adjusting _save_numpy

@astrofrog
Copy link
Member Author

I should clarify that ideally I would add a checkbox to the file save dialog but haven't looked into how to do this yet (instead of having the drop-down menu)

@astrofrog astrofrog changed the title Add ability to save zip bundle Add ability to include serialized data in glu file Jun 13, 2015
@astrofrog astrofrog added this to the 0.4.0 milestone Jun 13, 2015
@ChrisBeaumont
Copy link
Member

Changes look good to me

@astrofrog
Copy link
Member Author

Ok, thanks! I should probably add a unit test and a changelog entry before merging.

astrofrog added a commit that referenced this pull request Jun 19, 2015
Add ability to include serialized data in glu file
@astrofrog astrofrog merged commit 237330f into glue-viz:master Jun 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants