cyclops icon indicating copy to clipboard operation
cyclops copied to clipboard

Fix import_optional_module function to return NoneType

Open amrit110 opened this issue 9 months ago • 3 comments

PR Type ([Feature | Fix | Documentation | Test])

Fix

Short Description

The import_optional_module currently returns None when a module cannot be imported and the error parameter is set to ignore. When we use isinstance this doesn't work, so introducing an option to return NoneType.

Tests Added

...

amrit110 avatar Apr 29 '24 18:04 amrit110

Instead of returning sometimes returning NoneType, would it be better to check if it's None before isinstance?

yeah that sounds better. i wonder if the metric_dict is the only place where that needs to be done.

amrit110 avatar Apr 30 '24 16:04 amrit110

Instead of returning sometimes returning NoneType, would it be better to check if it's None before isinstance?

yeah that sounds better. i wonder if the metric_dict is the only place where that needs to be done.

There are a few cases of that in the models package. For example, in cyclops/models/torch_utils, there's

isinstance(X, (torch.Tensor, PackedSequence))

torch and PackedSequence are both optionally imported.

fcogidi avatar Apr 30 '24 17:04 fcogidi

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 75.14%. Comparing base (efb402d) to head (590b15f).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #597      +/-   ##
==========================================
- Coverage   75.15%   75.14%   -0.01%     
==========================================
  Files         128      128              
  Lines       11394    11399       +5     
==========================================
+ Hits         8563     8566       +3     
- Misses       2831     2833       +2     
Files Coverage Δ
cyclops/utils/optional.py 100.00% <100.00%> (ø)
...clops/evaluate/metrics/experimental/metric_dict.py 92.13% <50.00%> (-0.48%) :arrow_down:
cyclops/models/torch_utils.py 75.60% <50.00%> (-0.65%) :arrow_down:

Impacted file tree graph

codecov-commenter avatar Apr 30 '24 18:04 codecov-commenter