eagmon commented 1 year ago

This PR was previously opened with a different branch in PR #121. This PR fixes a long-standing NumPy error in which a slice object can no longer be used as an index, described in issue #123. Instead, I am pulling the ranges out of the slice object and using them directly.

I also fixed some listing errors and matplotlib deprecation errors, but left 4 tests still failing, down from 13 failing before this PR:

FAILED tests/model_lang/bngl/test_bngl_utils.py::BgnlUtilsTestCase::test_get_parameters_variables_for_simulation_with_empty_sample_times - AssertionError: “failed to parse action” does not match “Model file /home/runner/work/Biosimulators_utils/Biosimulators_utils/tests/model_lang/bngl/../../fixtures/bngl/empty-sample-times.bngl is not a valid BNGL or BNGL XML file. FAILED tests/sedml/test_sedml_validation.py::ValidationTestCase::test_validate_calculation - AssertionError: ‘The syntax’ not found in ‘- The mathematical expression a * is invalid.\n - unexpected EOF while parsing (, line 1)’ FAILED tests/combine/test_combine_exec.py::ExecCombineTestCase::test_1 - AssertionError: None != {‘sim.sedml’: {‘report1’: ‘ABC’, ‘report2’: ‘DEF’}} FAILED tests/combine/test_combine_exec.py::ExecCombineTestCase::test_2 - AssertionError: Lists differ: [‘log.yml’] != [‘dir1’, ‘log.yml’, ‘plots.zip’, ‘reports.zip’]

CodeByDrescher commented 1 year ago

I found an alternate solution to fix the numpy bug; turns out multi-dimensional indexing still works if its done using a tuple. We don't need the slices to be mutable, so a cast to tuple worked just fine for our purposes. Closes #123

eagmon commented 1 year ago

nice find @CodeByDrescher! Looks like the NumPy bug is fixed. There are still 4 tests failing, but maybe those should be addressed in a separate PR?

eagmon commented 1 year ago

@jcschaff -- there are 4 failing tests left. Maybe we divide and conquer? We could also do that in a separate PR if we just want to get the NumPy bug fixed now to unblock progress in the test suite.

codecov[bot] commented 1 year ago

Codecov Report

Base: 96.62% // Head: 96.64% // Increases project coverage by +0.02% :tada:

Coverage data is based on head (99db91d) compared to base (9355bad). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## dev #122 +/- ## ========================================== + Coverage 96.62% 96.64% +0.02% ========================================== Files 87 83 -4 Lines 9326 9297 -29 ========================================== - Hits 9011 8985 -26 + Misses 315 312 -3 ``` | Flag | Coverage Δ | | |---|---|---| | unittests | `96.64% <100.00%> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators) | Coverage Δ | | |---|---|---| | [biosimulators\_utils/model\_lang/bngl/validation.py](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#diff-Ymlvc2ltdWxhdG9yc191dGlscy9tb2RlbF9sYW5nL2JuZ2wvdmFsaWRhdGlvbi5weQ==) | `100.00% <ø> (ø)` | | | [biosimulators\_utils/combine/io.py](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#diff-Ymlvc2ltdWxhdG9yc191dGlscy9jb21iaW5lL2lvLnB5) | `89.94% <100.00%> (+0.10%)` | :arrow_up: | | [biosimulators\_utils/combine/validation.py](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#diff-Ymlvc2ltdWxhdG9yc191dGlscy9jb21iaW5lL3ZhbGlkYXRpb24ucHk=) | `97.47% <100.00%> (+0.02%)` | :arrow_up: | | [biosimulators\_utils/model\_lang/cellml/utils.py](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#diff-Ymlvc2ltdWxhdG9yc191dGlscy9tb2RlbF9sYW5nL2NlbGxtbC91dGlscy5weQ==) | `100.00% <100.00%> (ø)` | | | [biosimulators\_utils/report/io.py](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#diff-Ymlvc2ltdWxhdG9yc191dGlscy9yZXBvcnQvaW8ucHk=) | `100.00% <100.00%> (ø)` | | | [biosimulators\_utils/sedml/validation.py](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#diff-Ymlvc2ltdWxhdG9yc191dGlscy9zZWRtbC92YWxpZGF0aW9uLnB5) | `99.25% <100.00%> (-0.32%)` | :arrow_down: | | [biosimulators\_utils/viz/io.py](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#diff-Ymlvc2ltdWxhdG9yc191dGlscy92aXovaW8ucHk=) | `100.00% <100.00%> (ø)` | | | [biosimulators\_utils/ref/utils.py](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#diff-Ymlvc2ltdWxhdG9yc191dGlscy9yZWYvdXRpbHMucHk=) | `93.66% <0.00%> (ø)` | | | [biosimulators\_utils/sedml/math.py](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators#diff-Ymlvc2ltdWxhdG9yc191dGlscy9zZWRtbC9tYXRoLnB5) | `100.00% <0.00%> (ø)` | | | ... and [4 more](https://codecov.io/gh/biosimulators/Biosimulators_utils/pull/122/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=biosimulators)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.