Skip to content

237 add MedNIST dataset#263

Merged
Nic-Ma merged 7 commits intomasterfrom
237-add-MedNIST-dataset
Apr 14, 2020
Merged

237 add MedNIST dataset#263
Nic-Ma merged 7 commits intomasterfrom
237-add-MedNIST-dataset

Conversation

@Nic-Ma
Copy link
Copy Markdown
Contributor

@Nic-Ma Nic-Ma commented Apr 10, 2020

Fixes #237 .

Description

This PR added MedNIST dataset link to the tutorial.

Status

Ready

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or new feature that would cause existing functionality to change)
  • New tests added to cover the changes
  • Docstrings/Documentation updated

@wyli wyli force-pushed the 237-add-MedNIST-dataset branch 2 times, most recently from c7b4069 to 3f6f8a8 Compare April 10, 2020 17:57
Copy link
Copy Markdown
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you also check that the wget works @Nic-Ma

@wyli wyli force-pushed the 237-add-MedNIST-dataset branch from 3f6f8a8 to 079427d Compare April 10, 2020 20:30
@ericspod
Copy link
Copy Markdown
Member

Should we be using dropbox for this? Perhaps GIT LFS through github makes more sense. Also using the program tar makes this almost impossible to run on Windows, Python's tarfile library perhaps should be used to access the contents.

@wyli
Copy link
Copy Markdown
Contributor

wyli commented Apr 11, 2020

thanks, updated to use tarfile. i don't have any preference for the file hosting please feel free to change

@tvercaut
Copy link
Copy Markdown
Member

I agree that using git lfs with no tarball/zip or similar would be much better.

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Apr 13, 2020

Should we be using dropbox for this? Perhaps GIT LFS through github makes more sense. Also using the program tar makes this almost impossible to run on Windows, Python's tarfile library perhaps should be used to access the contents.

Hi @ericspod ,

Could you please help set the expected storage for this dataset? Or feel free to submit PR directly.
Both Wenqi and I don't have much experience with it.
Thanks.

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Apr 14, 2020

Hi @ericspod and @tvercaut ,

As we are closing to v0.1 release deadline, if we don't have a quick solution here, can we merge this PR first? At least users can get the link in the tutorial.
Thanks.

@tvercaut
Copy link
Copy Markdown
Member

The simplest would be to create a new MedNIST (or similar name) repo under Project-MONAI and enable git lfs on it (see https://help.github.com/en/github/managing-large-files). That said, it would be best to do this once the licensing question discussed in #237 has been resolved.

@ericspod
Copy link
Copy Markdown
Member

I would leave sorting LFS off until after the v0.1 release. It's not critical, just a best practice thing, and will make more sense the more examples with large scale data we add.

@Nic-Ma
Copy link
Copy Markdown
Contributor Author

Nic-Ma commented Apr 14, 2020

I would leave sorting LFS off until after the v0.1 release. It's not critical, just a best practice thing, and will make more sense the more examples with large scale data we add.

So do you agree to merge this PR first for v0.1 and add LFS support later?
Thanks.

@ericspod
Copy link
Copy Markdown
Member

@tvercaut mentioned a licensing issue, once that is resolved yes we merge.

@Nic-Ma Nic-Ma merged commit 2ed6b7d into master Apr 14, 2020
@wyli wyli deleted the 237-add-MedNIST-dataset branch May 21, 2020 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add descriptions for the MedNIST dataset

4 participants