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

Add support for URIs #302

Merged
merged 2 commits into from
Mar 3, 2021
Merged

Add support for URIs #302

merged 2 commits into from
Mar 3, 2021

Conversation

timholy
Copy link
Member

@timholy timholy commented Mar 3, 2021

User must load HTTP manually for this to be activated.
(It exploits Requires.)

Closes #219
Closes #89

User must load HTTP manually for this to be activated.
(It exploits Requires.)

Closes #219
@codecov
Copy link

codecov bot commented Mar 3, 2021

Codecov Report

Merging #302 (b9ca1a1) into master (1d9d382) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #302      +/-   ##
==========================================
+ Coverage   89.54%   89.59%   +0.05%     
==========================================
  Files          10       10              
  Lines         593      596       +3     
==========================================
+ Hits          531      534       +3     
  Misses         62       62              
Impacted Files Coverage Δ
src/FileIO.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d9d382...2c603ef. Read the comment docs.

@timholy timholy merged commit 79a49d4 into master Mar 3, 2021
@timholy timholy deleted the teh/uris branch March 3, 2021 19:20
@johnnychen94
Copy link
Member

johnnychen94 commented Mar 5, 2021

FWIW, ping @oxinabox since ReferenceTests requires FileIO and I know you don't like Requires :P

@timholy
Copy link
Member Author

timholy commented Mar 5, 2021

Out of curiosity, what don't you like? I used to dislike it too until we got the latency down...but oh, it's crept back up again. [works for a while] There, that's better: JuliaPackaging/Requires.jl#101

@SimonDanisch
Copy link
Member

Isn't Download.jl a std lib in 1.6, so basically free to depend on going forward - and also supposed to be the stable standard to download files?

@timholy
Copy link
Member Author

timholy commented Mar 5, 2021

Yes, but I'm not sure I understand. Are you saying we should move Requires to Download?

@SimonDanisch
Copy link
Member

No, use Download instead of HTTP and just depend on it without Requires... Although I just see, that we would also need to depend on URIParsers.jl in that case (but it's at least also a small, dependency free package).

@timholy
Copy link
Member Author

timholy commented Mar 5, 2021

Oh, interesting, I'd not noticed that. That seems like a good idea if we stop supporting anything younger than 1.6.

Indeed, one fortunate thing is that the most recent release of HTTP factored out their URI support into URIs.jl, and URIParsers.jl is deprecated. If we can rely on a minimum release that depends on URIs, we can make a seamless transition from HTTP to URIs.

If you have a dependency which might not be used:

tim@diva:~/src/julia-master/base$ juliam --startup-file=no -q
julia> @time using Example    # precompile the package-loading machinery
  0.047376 seconds (103.68 k allocations: 6.508 MiB, 89.74% compilation time)

julia> @time using URIs
  0.065603 seconds (102.69 k allocations: 6.830 MiB)

julia> 
tim@diva:~/src/julia-master/base$ juliam --startup-file=no -q
julia> @time using Example     # fresh session, do it again
  0.046348 seconds (103.68 k allocations: 6.508 MiB, 89.96% compilation time)

julia> @time using Requires
  0.052093 seconds (21.19 k allocations: 1.871 MiB)

but together with Requires' __init__ it goes to ~150ms (see demo in JuliaPackaging/Requires.jl#101).

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.

Support URLs and FilePaths Support URLs
3 participants