Remove --mask argument + restructure
#55
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added integration tests for hres_ic Added development environment
Before the actual test run, there is a setup step that sets up the work directory and data paths Currently the tests are not meant to be run in parallel yet, as the setup should be re-structured to be run only once (before the test run) and not for each parallel worker.
- Mock the shutil.move function so the outputs can be sent to a custom path and there is no need for copying the input data - Use fixtures - Use mark.parameterize - Use built-in tmp_path_factory fixture - Enable parallelized tests Tests can now be run with 'pytest -n <num> | auto' to run in parallel.
…e' to make it general. Also changed the entry points to only include replace_landsurface (old 'hres_ic').
…ce_landsurface' function
- Simplified code removing redundant portions - Deleted redundant MASK input - Turned the class bounding_box to the function get_bounding_box, that returns the BoundingBox namedtuple. Simplified the way to get boundary box extents. - Restructured the get_barra_data function to use xarray DataArray.sel method, together with slice and 'method' keyword for more efficient and simpler data retrieval.
… and make it consistent with the BARRA2R script Updated BARRA2R script
…d other redundant variables - General minor updates
atteggiani
force-pushed
the
davide/remove_mask
branch
from
2639cd8 to
40eb164
Compare
Updated gitignore
atteggiani
force-pushed
the
davide/remove_mask
branch
from
3ba4679 to
897295e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
This PR head branch was based off
davide/remove_hres_eccb(which includesdavide/integration_tests_pytest) to provide a way to test the scripts after their modification.It is suggested the branch to be rebased to
mainafter #53 and #59 are merged, before marking this PR as "ready to be reviewed".Overview
This PR main objective is to remove the redundant
--maskargument from thereplace_landsurfacescripts.In doing so, the code has been restructured the following issues ended up getting fixed.
List of the modifications and fixed issues
--maskargument. Fixes Is the MASK input parameter redundant? #51.BoundingBoxclass to theget_bounding_boxfunction. Fixes Turnbounding_boxclass to function #35.printstatements. Fixes Make print statements more informative for future debugging #7.sys.exitwithraise <exception>clauses. Fixes Replacesys.exit()statements with exception raises #26.&) withand#23 (Portion of code not included anymore in the new version).fnamewithncfname#10 (Line not included anymore in the new version).Testing
PR passes the integration tests with little needed modifications (removed the
--maskargument from the tests).Why is this important
This changes make the code cleaner and easier to test.
Important
SUITE MODIFICATIONS NEEDED
These changes break back-compatibility with this version of the RNS suite.
An updated version of the suite is available in this branch.