hdmf-dev / hdmf

The Hierarchical Data Modeling Framework
http://hdmf.readthedocs.io
Other
46 stars 26 forks source link

relative paths for external links #16

Closed bendichter closed 5 years ago

bendichter commented 5 years ago

Briefly describe the needed feature as well as the reasoning behind it

Is your feature request related to a problem? Please describe. I was trying to share two files, one of which has an external link to another, and I realized that this link used the absolute path to second file, not the relative path, so when I moved the two files the link broke, even if the relative position of the files stayed the same (they were in the same directory). This makes it pretty much impossible to share files that have links between them.

Describe the solution you'd like I would like to be able to use relative links instead, so that I can move both of the files at once and the link continues to work.

Additional context I am currently using external links to hold subject-specific information. I am working with ECoG data that contains the a cortical surface mesh object. My current solution is to save this mesh object in every session file, but sometimes there are 100+ session files for a single subject and this surface mesh object never changes for a subject, so it would be better to store this as a separate file and link to if from each session file.

Checklist

oruebel commented 5 years ago

I think that's a bug. I believe for Datasets relative paths should be created correctly, see:

https://github.com/hdmf-dev/hdmf/blob/99ce39e96cab7330d37e739efd0f6451eb3da1f0/src/hdmf/backends/hdf5/h5tools.py#L629-L634

I think the problem is for Groups in the HDF5IO.write_link function here:

https://github.com/hdmf-dev/hdmf/blob/99ce39e96cab7330d37e739efd0f6451eb3da1f0/src/hdmf/backends/hdf5/h5tools.py#L591

Similar to how external links are created for datasets the path to the file should be changed to relative path here when creating the ExternalLink. I have not tested this but I think the code should look like this:

elif target_builder.source is not None:
            target_path = os.path.relpath(path=os.path.abspath(target_builder.source), 
                                          start=os.path.abspath(parent.file.filename))
            link_obj = ExternalLink(target_path, path)
bendichter commented 5 years ago

good catch, @oruebel! That looks like the spot. I think relative paths should be default. Is there a reason to use absolute paths? If so, maybe we can create an option to use absolute paths instead.

oruebel commented 5 years ago

I think relative paths should be default.

Agreed. I think this was really just a bug we had missed.

Is there a reason to use absolute paths?

I can't think of a use-case where we would need absolute paths. If it comes up it's something that can be easily fixed with an additional option, but for now, I'd say we just use relative paths by default.