Ticket/PSB-207: #2726
Ticket/PSB-207: #2726
Conversation
morriscb
force-pushed
the
ticket/PSB-207/dev
branch
from
8b12025
to
629f3e6
Compare
| Return a list of all of the file names of the manifests associated | ||
| with this dataset | ||
| """ | ||
| return None |
There was a problem hiding this comment.
should this raise NotImplementedError instead?
There was a problem hiding this comment.
Nope. This is a function that gets called in the base class so some value needs to be there. I'll change up the comment.
I can look into getting something mocked up with moto. |
| """ | ||
| return None | ||
|
|
||
| def download_data(self) -> pathlib.Path: |
There was a problem hiding this comment.
Is this functionality different than the download_data in the base class?
There was a problem hiding this comment.
Slightly, the ordering is different in that things not used because the file metadata is set and doesn't need to be retrieved. So it's basically as simpler version of what is already in S3CloudCache.
There was a problem hiding this comment.
It seems that you instead want to override data_path and then the download_data of the superclass will just work? Want to try to get rid of the bit of boilerplate code duplication here.
| RuntimeError | ||
| If the file cannot be downloaded | ||
| """ | ||
| file_attributes = self._file_attributes |
There was a problem hiding this comment.
This can be improved further since there is still too much code duplication with superclass data_path
It looks like what needs to be done is in S3CloudCache, you can add
@property
def file_attributes(self):
return self._manifest.data_file_attributes(file_id)
then in NaturalMovieOneCache you can override with
@property
def file_attributes(self):
return self._file_attributes
and then get rid of data_path
| @@ -339,6 +350,29 @@ def data_path(self, file_id) -> dict: | |||
|
|
|||
| return output | |||
|
|
|||
| def get_file_attributes(self, file_id): | |||
There was a problem hiding this comment.
pretty sure it returns CacheFileAttributes and not dict ? Can you add return type?
There was a problem hiding this comment.
Ah, was looking at my natural movie cache and saw what looked like a dict at first glace. Fixed.
| """ | ||
| return None | ||
|
|
||
| def get_file_attributes(self, file_id): |
There was a problem hiding this comment.
I believe this returns CacheFileAttributes and not dict ? Can you add return type?
Add file attribute for download. Hook up natural movie into cache. Add natural movie processing. Add natural movie test. Add get_file_attributes method. Change return type.
morriscb
force-pushed
the
ticket/PSB-207/dev
branch
from
cd17b7b
to
c6202e6
Compare
No description provided.