-
Notifications
You must be signed in to change notification settings - Fork 80
Add FileDownloader class with file:// protocol #430
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
base: main
Are you sure you want to change the base?
Conversation
… e.g. on a Network share. Path needs to start with file://.
Hi @gyger. Thanks for opening this PR. Sorry for the delayed reply, I spent last week at a conference. I'll take a look at this soon. |
Hi @gyger! This new class looks good! I think it would be a nice addition to Pooch. A few things that we need to add before we can merge this PR:
It would also be nice to include an example in the documentation on use cases for this new downloader. Let me know if you can and are willing to address these points. Feel free to ask for help if you need it. |
I'm eagerly awaiting this PR merge - fantastic idea. I'm happy to help if needed for those final steps, but will await your thoughts @gyger, @santisoler. |
Please go ahead modifying the remaining open points. Was occupied with other stuff. |
OK, so I've forked and checked out the |
OK, so the final items are done, but just checking in first to potentially avoid creating a separate PR. I merged the origin to get up to date before starting work, then:
I guess there are two paths forward to merge:
Seems to me option 1 would be good, but which would you prefer @gyger? |
Hi @santisoler, |
Add FileDownloader class with file protocol to allows repository to be e.g. on a network share
Path needs to start with file://.
Relevant issues/PRs:
See #429