Skip to content

Conversation

@lsfischer
Copy link

This PR updates the path in create-dataset.sh to reflect that the result of wget + unrar stores the files in a SRTM_XX_250m_TIF/SRTM_XX_250m.tif path

Copy link

@charlycou charlycou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also about to submit a PR about this.
I would have unar the .rar files with the -D option

-D, -no-directory
Never create a containing directory for the contents of the unpacked archive.

Also the /code/data directory is not created if I'm not mounting a data directory from my host machine. We could create it at the begining of the script.

OUTDIR="/code/data"
if [ ! -e $OUTDIR ] ; then
    echo $OUTDIR does not exist! Creating it.
    mkdir $OUTDIR
fi

What do you think?
Should fix #50

@lsfischer
Copy link
Author

@charlycou Your solution sound good 👍

Actually instead of echoing and creating the directory we can create it directly if it does not exist with mkdir -p $OUTDIR will adjust to that

Copy link

@charlycou charlycou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lsfischer you forgot to cancel your modifications on create-dataset.sh

@ViperG
Copy link

ViperG commented Jul 15, 2022

Fyi this is still an issue when you use the pre-existing openelevation/open-elevation docker image (as the api docs instruct) and run it to generate the data tile sets. switching from unrar to unar in an older commit changes the file output behavior and adds an extra folder per .tar file so the gdalinfo commands fail since they have the incorrect path. Workaround of course is to not use the docker image and run the commands manually or import the docker image and replace the scripts and build a new docker image.

@lsfischer lsfischer requested a review from charlycou August 11, 2023 19:12
@lsfischer
Copy link
Author

Thanks a lot @charlycou!

@Jorl17 any interest in merging this? I'd be happy to help out on other open issues

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.

3 participants