search

eldang / elevation_lookups

Takes an input file of paths described as series of points, outputs a file of data about the elevation changes along those paths.
Apache License 2.0
3 stars 2 forks source link

Is GDAL really a dependency? #9

Closed eldang closed 3 years ago

eldang commented 3 years ago

I've listed GDAL as a dependency in the README, because rasterio claims to need it, and since it was already installed on my machine I never actually tested whether it was necessary. I just noticed that @dabreegster's Dockerfile doesn't seem to do anything to install GDAL. I've verified that dockerised use of this project does work, including in contexts in which it would use rasterio.

It's possible that either I've missed a way GDAL gets installed in Docker as a side-effect, or we're getting away with this simply because the way I'm using rasterio doesn't happen to hit any GDAL interactions.

@dabreegster do you happen to know? If it's the latter explanation, please don't fix this yet! It would be useful for us to know if at the end of main development GDAL still isn't really required, and if that turns out to be the case then I'll clarify that in the README. And if I later run into something that starts to require GDAL it won't be hard to add that to the Dockerfile as needed.

eldang commented 3 years ago

Update: I tested adding --no-binary rasterio to the pip install command in Docker, and using that makes rasterio installation fail with ERROR: A GDAL API version must be specified. Provide a path to gdal-config using a GDAL_CONFIG environment variable or use a GDAL_VERSION environment variable.

So GDAL isn't a real dependency for building the project! If my latest code changes work without installing it, I'll update the README.

dabreegster commented 3 years ago

I removed all datasources, forcing SRTM, and ran with the current Docker image. It looks like GDAL is needed.

curl -s -o spool/N47/N47W123.hgt.gz.temp https://s3.amazonaws.com/elevation-tiles-prod/skadi/N47/N47W123.hgt.gz && mv s
pool/N47/N47W123.hgt.gz.temp spool/N47/N47W123.hgt.gz
gunzip spool/N47/N47W123.hgt.gz 2>/dev/null || touch spool/N47/N47W123.hgt
gdal_translate -q -co TILED=YES -co COMPRESS=DEFLATE -co ZLEVEL=9 -co PREDICTOR=2 spool/N47/N47W123.hgt cache/N47/N47W123.tif 2>/dev/null || touch cache/N47/N47W123.tif
rm spool/N47/N47W123.hgt
make: Leaving directory '/root/.cache/elevation/SRTM1'
make: Entering directory '/root/.cache/elevation/SRTM1'
gdalbuildvrt -q -overwrite SRTM1.vrt
make: gdalbuildvrt: Command not found
Makefile:14: recipe for target 'SRTM1.vrt' failed
eldang commented 3 years ago

Ohhhhh.... iiiinteresting. So it looks like in practice GDAL is a required dependency for the elevation module whether it is for rasterio or not. I suspect that I was getting away with that in my testing yesterday because I already had the SRTM tiffs downloaded to the /data directory I'd mounted. Next time I dependency test I'll empty that directory....

eldang commented 3 years ago

Sure enough, https://github.com/bopen/elevation lists GDAL as a dependency. I should have checked that yesterday....

eldang commented 3 years ago

Reclosing this because the README is now back to calling GDAL a dependency.