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

elevation package #9

Closed
emanuel-schmid opened this issue Mar 5, 2020 · 4 comments · Fixed by #97
Closed

elevation package #9

emanuel-schmid opened this issue Mar 5, 2020 · 4 comments · Fixed by #97

Comments

@emanuel-schmid
Copy link
Collaborator

emanuel-schmid commented Mar 5, 2020

The elevation package is used by the climada.hazard.tc_surge module and also imported in methods of climada.util and climada.hazard.centroid.

As for now, this package can be well installed on a Windows computer through pip. But it will not work, not in version 1.0.6 in any case.

The reason why it doesn't work is that in the background it basically runs bash or bash commands.

  • make can be installed with conda
  • unzip can be globally installed with UnxUtils
  • mv dito
  • find dito, but the PATH must be set before the winows command find
  • mkdir mkdir from UnxUtils won't work because option -p is ignored, but can easily be implemented. However there seems to be no way mkdir from Windows can be overridden (probably because it is an "internal command").

Bottomline: elevation must be adapted to work for Windows. Alternatively we relinquish its use and switch to another package or simply implement the required functionality.

@emanuel-schmid
Copy link
Collaborator Author

All dependencies from elevation have been removed from the master branch.
In order to allow seamless merging of the develop branch onto the master branch (e.g. for making releases) I moved all these dependencies into the separated, dedicated tcsurge branch.

@chahank
Copy link
Member

chahank commented Aug 3, 2020

Opened an issue on the elevation package repository concerning the mkdir functionality problem. Proposed to fix the problem by using mkdir without flag if windows is detected. The answer by the developers (link):

"I'm sorry the tools has been design to work on Linux/MacOS/Unix and I never tested it on Windows. It may work using the Windows Subsystem for Linux (WSL), but it is not supported."

"You might consider filing a bug with windows that it is not implementing the -p option which is required by POSIX: https://pubs.opengroup.org/onlinepubs/9699919799/

While that seems hard, it would probably make a lot of things work better on Windows if it were fixed."

@emanuel-schmid
Copy link
Collaborator Author

emanuel-schmid commented Aug 10, 2020

WSL is not really an option, because

  • currently WSL and conda don't work together well when it comes to packages with C extensions
  • even if this was fixed, asking our users to use WSL would equal to abandon Windows support.

I agree on your adjustment that filing a bug report to Windows will lead nowhere. (I even wonder whether that suggestion was actually sarcastic.)

Which leaves 3 options:

  • fork the elevation package, hack it to make it work for windows and then submit a pull request. If it's rejected we may need to keep the fork in sync.
  • fork the elevation package and replace all the make stuff with python functions. Call it elvation2 (pure python). Then forget about it.
  • once more look for an existing replacement.

@chahank
Copy link
Member

chahank commented Sep 18, 2020

A more sophisticated tc_surge model is being developed which does not rely on the elevation package. Furthermore, surge models are very sensitive to the local geometry (elevation). Hence, it may be better to not provide generic elevation data, and instead, require the user to provide such data.

Suggestion: remove the elevation package from the requirements and archive the current tc_surge module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants