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

refactored extents algorithm #89

Merged
merged 7 commits into from
Aug 23, 2024
Merged

refactored extents algorithm #89

merged 7 commits into from
Aug 23, 2024

Conversation

erialC-P
Copy link
Collaborator

This PR adds in the refactored extents algorithm to our production code workflow.

I have:

  • replaced the contents of the former extents.py with the new functions and workflow prototyped in notebooks/experimental/Extents_experiments.ipynb
  • moved the function ocean_mask from the former extents.py script into elevation.py
  • unfortunately, this may have broken the application of the ocean_mask func in the elevation function. For now, I have defaulted back to 'don't ocean mask' in the elevation call in the CLI function to get around this. I believe that this masking will be replaced by the new connectivity workflow in a future PR which is why I didn't investigate the bug further.
  • I also did not create functionality to return the connectivity mask as part of this workflow, believing that might be more easily achieved in a future PR when connectivity is built into the elevation function.

Following this PR, I think we're ready to either:

  • a) conduct a broadscale extents test on a select series of tiles or;
  • b) move the connectivity function out of extents.py and into the elevation function in elevation.py prior to broadscale testing.

@erialC-P erialC-P requested a review from robbibt August 22, 2024 01:12
Copy link

For full integration test results, refer to the Tests directory README.

@robbibt robbibt merged commit 4d8b199 into develop Aug 23, 2024
@robbibt robbibt deleted the extents_refactor branch September 24, 2024 06:07
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.

2 participants