make default fetch fallback to Base.download #103
Conversation
Codecov Report
@@ Coverage Diff @@
## master #103 +/- ##
==========================================
- Coverage 64.86% 64.58% -0.29%
==========================================
Files 12 12
Lines 185 192 +7
==========================================
+ Hits 120 124 +4
- Misses 65 68 +3
Continue to review full report at Codecov.
|
|
Maybe add to readme/ docstring for DataDep that if remote_path is a type that supports |
| See also [`fetch_base`](@ref) and [`fetch_http`](@ref). | ||
| """ | ||
| function fetch_default(remotepath, localdir) | ||
| if remotepath isa AbstractString && occursin(r"^https?://", remotepath) |
There was a problem hiding this comment.
Maybe include a link to the discussion on base as to why these are different?
There was a problem hiding this comment.
Not really relevent I think.
Real difference is wither we need to use Base.basename or that we have a download that can get it right on its own.
Not so much about joinpath stuff
test/fetch_helpers.jl
Outdated
| # This is easy because the filename is in the URL | ||
| # So it works with both `fetch_base` and `fetch_http` | ||
| # HTTP.jl has tests for much more difficult cases, and fetch_http supports those | ||
| @testset "$fetch" for fetch in (fetch_default, fetch_base, fetch_http) |
There was a problem hiding this comment.
Pretty minor, but I might be inclined to rename fetch to f or func to make it clear that we aren't trying to call the base fetch method in this loop.
Co-Authored-By: Rory Finnegan <rory.finnegan@gmail.com>
This should close #100,
The idea is that if a type has overloaded
Base.downloadandBase.basename,then that is generally enough for us to know what to do.
@rofinn can you review?
I'm tagging as a breaking change as I also want to drop deprecations.