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

Refactor the Pooch class to make it easily subclassed #27

Closed
leouieda opened this issue Sep 13, 2018 · 4 comments · Fixed by #66
Closed

Refactor the Pooch class to make it easily subclassed #27

leouieda opened this issue Sep 13, 2018 · 4 comments · Fixed by #66
Labels
enhancement Idea or request for a new feature
Milestone

Comments

@leouieda
Copy link
Member

From @remram44 comment on #4, it would be great to expose more of the inner workings of the Pooch class as methods. This would make it easy to subclass it and overwrite those methods. The _download_file method should also be made public (remove the leading _).

Any ideas and suggestions on what should be refactored are welcome.

@leouieda leouieda added enhancement Idea or request for a new feature help wanted labels Sep 13, 2018
@leouieda leouieda added this to the v1.0.0 milestone Sep 13, 2018
@santisoler
Copy link
Member

Would be great to have an easier access to the filenames inside the registry.
Currently, we must search for them inside the registry dictionary, but we must convert the dict_keys to list first. This leads to ugly coding.

For example, if we want to get a list of every filename in the registry we must do something like this:

filenames = list(GOODBOY.registry)

Would be nice to have a more stylish way of doing it, something like GOODBOY.filenames.

@leouieda
Copy link
Member Author

@santisoler OK, that's something we could easily add as a property in Pooch. Maybe registry_files instead of filenames. Would you mind opening a PR for this?

@santisoler
Copy link
Member

@leouieda I've just opened #42 for this improvement.

@leouieda
Copy link
Member Author

Thanks! Merged

leouieda added a commit that referenced this issue May 1, 2019
Refactor the actual HTTP download into a "downloader" class. 
Allow custom downloaders to be passed to `Pooch.fetch`.
The advantage of this over subclassing `Pooch` is that it requires less effort from users.
We can also provide a collection of ready-made downloader classes.
For now, we have `HTTPDownloader` which can be used to pass credentials to the server.

Fixes #65 
Fixes #27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Idea or request for a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants