georust / proj

Rust bindings for the latest stable release of PROJ
https://docs.rs/proj
Apache License 2.0
137 stars 43 forks source link

Port from reqwest to ureq #205

Closed TomFryersMidsummer closed 1 month ago

TomFryersMidsummer commented 1 month ago

Addresses #189, reducing cargo tree -F network | wc -l from 296 to 190.

urschrei commented 1 month ago

Thanks for this @TomFryersMidsummer! As I note in the issue, I don't consider the proj network use case as requiring async http, so reqwest was not a conscious choice and I'm happy to see it replaced.

TomFryersMidsummer commented 1 month ago

Thank you for the feedback. I believe I've addressed everything now

lnicola commented 1 month ago

I believe _network_read_range still returns the Content-Lengt instead of the actual size of the data.

TomFryersMidsummer commented 1 month ago

Yes, sorry – missed that. I’ve fixed it now.

lnicola commented 1 month ago

Looks good, at least looking at the new commits, but I'll let someone else merge it since I've never used this crate.

urschrei commented 1 month ago

I also don't use this feature of proj and hope we can just remove it someday

The network feature? Hard to imagine it going anywhere: the grid download functionality is a big step forward; without it you have to ship a couple of hundred MBs around with every libproj (which we wouldn't be able to do bc of crates.io limitations, even if we wanted to), and older versions have out-of-date grids leading to less-accurate conversions etc.

frewsxcv commented 1 month ago

Oops maybe I'm mixing up the network functionality in a different crate, disregard!