Closed nikk-nikaznan closed 5 months ago
Attention: Patch coverage is 26.26932%
with 334 lines
in your changes missing coverage. Please review.
Project coverage is 39.60%. Comparing base (
e247e89
) to head (380da85
).
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi @nikk-nikaznan,
I liked this organisation, feels clearer!
I had a go at changing a few things - below an attempt at explaining my thinking. Let me know thoughts.
detection
module ---> detector
.tracking
module ---> tracker
.This is to avoid repetition between higher and lower level modules.
crabs.detector.utils
submodule.
_utils
modules.detection_utils
and evaluation_utils
into:
crabs.detector.utils.detection
: utils shared by evaluation and training, crabs.detector.utils.train
: utils only used in training,crabs.detector.utils.evaluate
: utils only used in evaluation.optuna
utils there (named optuna
for now, but we can discuss!) and the visualization
.Initially I wasn't sure if it would be too much nesting (mainly wanted to try). But I think it leaves the crabs.detector
level a bit more clear at a glance. It may help writing unit tests too.
In the tracker
module I did some more substantial changes. I tried to follow the structure of crabs.detector
.
crabs.tracker.sort
module, which only has the tracker classes
crabs.detector.models
).
crabs.tracker.evaluate_tracker
module with a TrackerEvaluate
class crabs.detector.evaluate_model
and the DetectorEvaluate
class).crabs.tracker.track_video
module which would be the main entry point crabs.detector.train_model
or crabs.detector.evaluate_model
).crabs.tracker.utils
submodule, which holds
sort
utils: I extracted them from the sort script,tracking
utils: the previous tracking_utils
,io
utils. This is the most substantial change. The io
module contains what was called before inference
, removing the class. The class was called Inference but it seemed mostly io operations (there was no prediction or tracking in the class), so I thought this format could work better.I think having a separate config is a good idea!
I also changed the entry points name for the tracker to something a bit more explicit (albeit longer): detect-and-track-video
.
I did my best but I am les familiar with the tracking bits, so there may be fixes required in the tracking 🙈
Thank you Sofia! Looking good. I just fixed some tracking bits that throw error. I am happy with the new name convention, I just tried to follow the module name as in the issue, but have no strong opinion on either. The only thing, can we not rename optuna_util
as optuna
? We can either do _optuna
or hyperparameter_optimizer
or hpo
?
After #141 Another attempt of #106 Probably first attempt towards #190