Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite sample_data.py as a class #121

Closed
niksirbi opened this issue Feb 21, 2024 · 2 comments · Fixed by #171
Closed

Rewrite sample_data.py as a class #121

niksirbi opened this issue Feb 21, 2024 · 2 comments · Fixed by #171
Labels
enhancement New optional feature

Comments

@niksirbi
Copy link
Member

Is your feature request related to a problem? Please describe.
Currently the functionalities for fetching sample datasets are implemented in the sample_data.py module. This works well but the interface results in some awkwardly worded and repetitive expressions, e.g.:

from movement import sample_data

sample_data.list_sample_data()
file_path = sample_data.fetch_sample_data_path("filename")
ds = sample_data.fetch_sample_data("filename")

Describe the solution you'd like
Perhaps it's better to refactor the functionality into a class named SampleData and have the interface look sth like below:

from movement import SampleData

sample_data = SampleData(local_data_dir)
sample_data.list()
file_path = sample_data.fetch_path("filename")
ds = sample_data.fetch("filename")

The current global variable DATA_DIR could be initialised as class attributes local_data_dir with the default value ~/.movement/data, but giving the user the option to send it elsewhere.
The global DATA_URL could be made into a class variable with a fixed value.

Context
This is not critical or urgent, just wondering which interface feels more intuitive and consistent to people.

@niksirbi
Copy link
Member Author

niksirbi commented Mar 8, 2024

Look into how scikit-learn, scikit-image, napari, and nilearn do it.

@niksirbi
Copy link
Member Author

niksirbi commented Mar 8, 2024

Consider SampleDataRegistry as a name for the class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New optional feature
Projects
Development

Successfully merging a pull request may close this issue.

1 participant