Generate mod_def.ww3 input file on the go#442
Generate mod_def.ww3 input file on the go#442ezhilsabareesh8 wants to merge 5 commits intopayu-org:masterfrom
Conversation
|
Hello @ezhilsabareesh8! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:
|
|
What's the status of this @ezhilsabareesh8 ? |
|
Thanks @anton-seaice. This PR became stale after the original review requests and I did not get follow-up feedback, so it has not moved forward since then. Since it is now far behind |
|
Thanks @ezhilsabareesh8 - ill give it a first review and then we can sync it up |
anton-seaice
left a comment
anton-seaice
left a comment
There was a problem hiding this comment.
Some general comments @ezhilsabareesh8
Is is slow to run this everytime payu setup is run ?
|
|
||
| # Run the ww3_grid command using the defined path | ||
| cmd = os.path.join(exec_dir, "ww3_grid") | ||
| result = os.system(cmd) |
There was a problem hiding this comment.
I think you need to check here that ww3_grid exists, and return an error otherwise
And check that result is successful, and return an error otherwise
There was a problem hiding this comment.
Updated in this commit dcd7f66 , to validate that ww3_grid exists and is executable before running it, and to raise a error if the command fails or does not produce mod_def.ww3.
| else: | ||
| print("ww3dev component not found in self.components.") |
There was a problem hiding this comment.
We don't want this print statement, most of the time ww3 isn't present
There was a problem hiding this comment.
Fixed in this commit dcd7f66 , WW3 setup now only runs when ww3dev is actually present
| os.chdir(original_dir) | ||
|
|
||
| # Check if ww3_grid command executed successfully | ||
| if result == 0: |
There was a problem hiding this comment.
This check should be at line 195 - it would be easier to understand there
| print("mod_def.ww3 generated successfully.") | ||
| else: | ||
| print("Error generating mod_def.ww3 file.") | ||
| raise SystemExit(1) # Terminate the program with exit code 1 |
There was a problem hiding this comment.
raise and Error with a message would be better
e.g.
raise RuntimeError("Failed to generate WW3 input files")
?
anton-seaice
left a comment
anton-seaice
left a comment
There was a problem hiding this comment.
This looks ok to me - be good to get someone from the release team to look over it
Co-authored-by: Anton Steketee <79179784+anton-seaice@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #442 +/- ##
==========================================
- Coverage 65.28% 65.16% -0.13%
==========================================
Files 68 68
Lines 5710 5727 +17
==========================================
+ Hits 3728 3732 +4
- Misses 1982 1995 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
This PR addresses part of issue #440 by implementing a feature to generate the mod_def.ww3 file on the go.
mod_def.ww3on the go.