search

rice-solar-physics / pydrad

Python tools for setting up HYDRAD runs and parsing output
https://pydrad.readthedocs.io
MIT License
4 stars 3 forks source link

Fix Windows compatibility in configure.py #140

Closed jwreep closed 3 years ago

jwreep commented 3 years ago

Fixes #139

Fixes the use of chmod and ., both of which are Unix-based commands and invalid on Windows. I tested this on Windows 10 and then verified on MacOS that it doesn't break anything.

wtbarnes commented 3 years ago

The chmod conditional looks good to me. As far as prepending a ./ goes, I wonder if this logic could just go in the run_shell_command script? I'm just reluctant to make the setup_intitial_conditions and setup_hydrad methods even more hard to follow than they already are and moving that logic to a separate function seems preferable.

So for example, you could just have a check for Windows and then if the command starts with a ./ and you're on windows, strip those characters.

jwreep commented 3 years ago

I've changed it as suggested. It appears to be working on windows. The one concern is that the modification to util.py is not terribly clean -- there's probably an easier-to-read way to write cmd = [cmd[0][2:]] to remove the leading ./, but it gets the job done. Any suggestion there?

jwreep commented 3 years ago

Right, I've moved on_windows to util.py. I tested on Windows without issue. It might be worth double-checking on Unix, too.

wtbarnes commented 3 years ago

Right, I've moved on_windows to util.py. I tested on Windows without issue. It might be worth double-checking on Unix, too.

I just made a few last minor changes and tested on Linux and it all looks good!