fix read(io) to support Downloads.download of stdlib#90
Conversation
```julia
using GeoJSON, Downloads
io = Downloads.download("https://its-live-data.s3.amazonaws.com/datacubes/catalog_v02.json", IOBuffer())
fc = GeoJSON.read(io)
```
using Downloads instead of HTTP.jl We don't need a dependency on HTTP.jl
| - `numbertype::DataType=Float32`: Use Float64 when the precision is required. | ||
| """ | ||
| function read(io; lazyfc=false, ndim=2, numbertype=Float32) | ||
| eof(io) && seekstart(io) |
There was a problem hiding this comment.
Thanks for the PR!
This also applies to strings and Vector{UInt8} as written now, so the code should be abstracted out somewhere else. Perhaps a separate dispatch of read for ::IO objects? CI is failing for this reason.
There was a problem hiding this comment.
Also needs some explanatory comments.
There was a problem hiding this comment.
Is it common to do such a thing? I've never seen it. It seems more up to the caller that the IOBuffer should not be at the end.
There was a problem hiding this comment.
Is it common to do such a thing? I've never seen it. It seems more up to the caller that the IOBuffer should not be at the end.
It's common to download GeoJSON data from web links when using GeoMakie
There was a problem hiding this comment.
Yeah, I agree, I meant specifically the seekstart. That seems like it is not the responsibility of that function.
There was a problem hiding this comment.
I agree that it is the responsibility of the caller to ensure the IO is in the usable status.
But check eof and seekstart will made GeoJSON.jl be user-friendly for beginners, especially for those who are unfamiliar with input/output streams.
There was a problem hiding this comment.
Yeah I appreciate the effort for user-friendliness, but am afraid that then users will expect other functions reading streams do the same, and possibly end up more confused. To keep the example simple it is also fine to just use Downloads.download("https://its-live-data.s3.amazonaws.com/datacubes/catalog_v02.json") to get a temp file path.
There was a problem hiding this comment.
maybe we can providing an seekstart bool argument and/or code example to facilitate this usage scenario.
4 participants
using Downloads instead of HTTP.jl
We don't need a dependency on HTTP.jl