Tarfiles #27
Tarfiles #27
Conversation
torchxrayvision/datasets.py
Outdated
| def __init__(self, imgpath): | ||
| if imgpath.endswith(".tar"): | ||
| self.tarred = tarfile.open(imgpath) | ||
| self.tar_paths = self.tarred.getmembers() |
There was a problem hiding this comment.
So you said this takes a lot of time. I see how this approach is super robust. I did some tests for time and it seems like just 30 seconds for the MIMIC data. What about caching this using a dict based on the file path? It would speed things up if multiple objects are created? But it seems like a reasonable price to pay.
The alternative is to skip it and assume a file structure but that is not as nice.
There was a problem hiding this comment.
Ok, using a dict, the second load is around 10x faster on my machine.
The dict could also be pickled so there is only one slow load. That option would lead to issues if someone wanted to change the tarfile, although I don't know why they would do that.
There was a problem hiding this comment.
Doing hashing and caching a file could be nice but also could be annoying to debug and create more issues (like if there are no write permissions).
torchxrayvision/datasets.py
Outdated
|
|
||
| class TarDataset(Dataset): | ||
| def __init__(self, imgpath): | ||
| if imgpath.endswith(".tar"): |
There was a problem hiding this comment.
What about using tarfile.is_tarfile ? This will allow for compressed files which people may want to use.
torchxrayvision/datasets.py
Outdated
| #print(img_path) | ||
| img = imread(img_path) | ||
| img = self.get_image(imgid) |
There was a problem hiding this comment.
You added this for NIH_Dataset but it doesn't extend TarDataset.
There was a problem hiding this comment.
Ah, let me fix that
|
Ah, ok. I agree, it was just a temporary thing because I have only tested this with MIMIC so far. I can fix it the other way and make a quick test |
|
I was running into some weird multiple inheritance issues when I tried creating a ZipDataset class and then tried having datasets that can be initialized from both. I think that it would be easier to extend to zipfiles, etc. if instead of having a TarDataset class, there was a "storage interface" object that takes a file path in the constructor and has a .get_image() method returning a numpy array. Then the dataloaders could all inherit from a class that picks the right type of storage interface, like this: Instead of having a self.get_image() method, the datasets could just call self.interface.get_image(). |
…t_of_date variable to prevent reading from an out-of-date cache
…meter for length of all datasets
… between sessions)
This pull request adds support for opening from tarfiles, zipfiles, and folders with any combination of tarfiles and/or zipfiles, whether in DICOM format or regular image formats such as PNG, and whether using a serial or parallel dataloader.
TODO: