Register test data as SampleDataSource #63
Conversation
Remove the copy of local data and instead register a SampleDataSource to be downloaded as needed. Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
jcfr
changed the title
| category='ShapeVariationAnalyzer', | ||
| uris='https://data.kitware.com/api/v1/item/626850ed4acac99f42121c7f/download', | ||
| loadFiles=True, # Unzip it | ||
| fileNames='ShapeVariationAnalyzerSampleData.zip', |
There was a problem hiding this comment.
Few comments:
-
There already a file called
SVASampleData.zipon the server. It is available at https://data.kitware.com/#collection/586fbb7b8d777f05f44a5c7b/folder/5d5c195085f25b11ff3e1ad8 -
After looking at the SHA512 of the one associated with the
SampleDataSource, I realized that the file downloaded is instead in a Public folder associated with a user (see screenshot below). We should avoid doing and instead consistently organize the zip files. -
The zip files should all be versioned (e.g
SVASampleData-YYYY-MM-DD.zip
| filepath_in = f"{base_dir}test.csv" | ||
| with open(filepath_in, "w") as csv_file: | ||
| csv_file.write("VTK Files,Group\n") | ||
| for vtk_file in vtk_files: | ||
| csv_file.write(f"{vtk_file},0\n") |
There was a problem hiding this comment.
Crafting the csv file here is counter productive, any one who would like to try to load the datasets independently would have to manually recreate the file.
Instead, the csv file should be provided and this issue #64 should be addressed.
| # ShapeVariationAnalyzerSampleData | ||
| # | ||
|
|
||
| class ShapeVariationAnalyzerSampleData(ScriptedLoadableModule): |
There was a problem hiding this comment.
Instead of introducing a new module ... may be worth adopting the style of the template. See https://github.com/Slicer/Slicer/blob/ab0aa85f93e226f40250857ec1c0b09304c4ea16/Utilities/Templates/Modules/Scripted/TemplateKey.py#L45-L50
And considering the reuse of the the sample data directly from SlicerSALT
|
@jamesfishbaugh Since the previous pull request was submitted independently so that it could be reviewed, I forced pushed master and re-open a new one. |
Remove the copy of local data and instead register a
SampleDataSourceto be downloaded as needed.