cyclops
cyclops copied to clipboard
Fix import_optional_module function to return NoneType
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
...
Instead of returning sometimes returning
NoneType
, would it be better to check if it'sNone
beforeisinstance
?
yeah that sounds better. i wonder if the metric_dict is the only place where that needs to be done.
Instead of returning sometimes returning
NoneType
, would it be better to check if it'sNone
beforeisinstance
?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.
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
@@ 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: |