Add option(s) to do automatic cleanup of "old" models/predictions/... #169
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #169 +/- ##
==========================================
+ Coverage 67.32% 69.91% +2.59%
==========================================
Files 23 25 +2
Lines 3204 3344 +140
==========================================
+ Hits 2157 2338 +181
+ Misses 1047 1006 -41 ☔ View full report in Codecov by Sentry. |
theroggy
requested changes
Mar 26, 2024
There was a problem hiding this comment.
Thanks! Good start!
Some general remarks that are best handled before doing a more detailed review:
- in general, application "logic" in orthoseg belongs in the /orthoseg/lib/ modules. So it would be best if cleanup.py would be moved there. Important: as a general principal, the modules in /orthoseg/lib/ offer a plain python API, so standard functions that get all necessary input via standard function arguments. In other words, the configuration (
conf.) is not accessed there, this happens in /orthoseg/train.py, /orthoseg/predict.py,... Hence: cleanup.py should follow this principle as well and rather than reading the config files it should use standard functions with arguments that can be called from e.g. /orthoseg/predict.py, where the configuration has been loaded already anyway. - the cleanup sample project is to large to include in the github repo. I think it will be more efficient as well as easier to maintain to create the test data in code by creating empty files as needed to test the cleanup logic.
Co-authored-by: Pieter Roggemans <pieter.roggemans@gmail.com>
- create cleanup sample dummy files - refactor cleanup test
|
Sample project removed and files are now created in code |
theroggy
requested changes
Mar 29, 2024
- renamed parameter path to model_dir - removed logging initialisation in each function
- test seperated in test for models, training en predictions
- refactor cleanup_old_data
theroggy
requested changes
Apr 16, 2024
- add test_cleanup_projects_dir - refactor cleanup tests
theroggy
requested changes
Apr 22, 2024
- more specific exceptions - refactor test_cleanup.py
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
resolves #52