Implement soft link and external link #87
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #87 +/- ##
==========================================
+ Coverage 95.57% 95.71% +0.13%
==========================================
Files 11 12 +1
Lines 1604 1702 +98
==========================================
+ Hits 1533 1629 +96
- Misses 71 73 +2
Continue to review full report at Codecov.
|
| @@ -71,7 +71,7 @@ class File(Group): | |||
| def __init__(self, directory, mode=None, allow_remove=False, | |||
| name_validation=None, plugins=None): | |||
| self._open_datasets = weakref.WeakValueDictionary({}) | |||
| directory = pathlib.Path(directory) #.resolve() | |||
| directory = pathlib.Path(directory).absolute() #.resolve() | |||
There was a problem hiding this comment.
Why is this necessary? Not that I mind, just good to know why it was added.
| @@ -220,6 +220,12 @@ def __getitem__(self, name): | |||
| return self | |||
| return super(File, self).__getitem__(path) | |||
|
|
|||
| def __setitem__(self, name, value): | |||
There was a problem hiding this comment.
Is this related to links or is it just something we have left out from before? Seems like this is something that has been missing on File regardless of links?
| @@ -413,6 +420,25 @@ def __getitem__(self, name): | |||
| ) | |||
| raise NotImplementedError(error_string) | |||
|
|
|||
| def _link(self, name, get_link=False): | |||
| link_meta = self._group(name).meta[exob.EXDIR_METANAME][exob.LINK_METANAME] | |||
| print(link_meta) | |||
| @@ -2,6 +2,15 @@ | |||
| EXDIR_METANAME = "exdir" | |||
| TYPE_METANAME = "type" | |||
| VERSION_METANAME = "version" | |||
| LINK_METANAME = "link" | |||
There was a problem hiding this comment.
I think I prefer if the type is soft_link, external_link, etc. That reduces the level of conditional nesting - you can say if type == "external_link" instead of if type == "link" and link_type == "external". And then have the meta-groups be soft_link and external_link as well. It just reduces the confusion of which variables can go together. (No-one will think that they can just change the target from external to soft and still keep the file, for instance.
| f = setup_teardown_file[3] | ||
| g = File(external_path, 'w') | ||
| f['ext'] = ExternalLink(external_path, '/missing') | ||
| with pytest.raises(KeyError): |
There was a problem hiding this comment.
To add to the comment above: This should not be a KeyError. It should actually return the object that the link is pointing to.
| @@ -509,20 +542,22 @@ def __len__(self): | |||
| assert_file_open(self.file) | |||
| return len([a for a in self]) | |||
|
|
|||
| def get(self, key): | |||
| def get(self, name, get_link=False): | |||
There was a problem hiding this comment.
I think the getter should first try to see if it is an object or a link, instead of forcing the user to tell it to expect a link or not. This is also how h5py works.
Should fix #1
Aims for compatibility with NWB NeurodataWithoutBorders/pynwb#629