Closed jwreep closed 2 months ago
Attention: Patch coverage is 79.54545%
with 9 lines
in your changes are missing coverage. Please review.
Project coverage is 92.43%. Comparing base (
73e3228
) to head (eee3efd
).
Files | Patch % | Lines |
---|---|---|
fiasco/io/sources/ion_sources.py | 50.00% | 7 Missing :warning: |
fiasco/conftest.py | 85.71% | 2 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I'm guessing CodeCov's issue is because the tests are for v8.0.6 and this requires < 8 to test?
I believe that's correct. There is code being added, but the tests that cover those lines are not being run on the CI.
Out of curiosity, do the tests pass if you run them against the v7 database? i.e.
pytest fiasco --ascii-dbase-root ~/.chianti --ascii-dbase-url http://download.chiantidatabase.org/CHIANTI_v7.1.4_database.tar.gz --disable-file-hash --skip-version-check
If they do, we should add an additional test to the CI for v7.
They do not pass, but it looks like it's because all of the .scups
files are missing:
=========================== short test summary info ============================
FAILED fiasco/io/sources/tests/test_sources.py::test_ion_sources[h_1.scups] - fiasco.util.exceptions.MissingASCIIFileError: Could not find file /Users/re...
FAILED fiasco/tests/test_collections.py::test_two_photon[wavelength0] - IndexError: index 0 is out of bounds for axis 0 with size 0
FAILED fiasco/tests/test_collections.py::test_two_photon[wavelength1] - IndexError: index 0 is out of bounds for axis 0 with size 0
FAILED fiasco/tests/test_collections.py::test_radiative_loss - assert False
FAILED fiasco/tests/test_collections.py::test_spectrum - ValueError: No collision or transition data available for any ion in collec...
FAILED fiasco/tests/test_ion.py::test_level - assert <Quantity 0> == 5
FAILED fiasco/tests/test_ion.py::test_proton_collision - assert False
FAILED fiasco/tests/test_ion.py::test_level_populations - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_level_populations_proton_data_toggle - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_contribution_function - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_emissivity_shape[density0-False] - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_emissivity_shape[density1-False] - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_emissivity_shape[None-True] - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_coupling_unequal_dimensions_exception - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_emissivity - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_intensity[em0] - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_intensity[em1] - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_intensity[em2] - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
FAILED fiasco/tests/test_ion.py::test_two_photon - IndexError: index 0 is out of bounds for axis 0 with size 0
FAILED fiasco/tests/test_ion.py::test_transitions - assert False
ERROR fiasco/tests/test_ion.py::test_level_populations_normalized - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
ERROR fiasco/tests/test_ion.py::test_level_populations_correction - fiasco.util.exceptions.MissingDatasetException: _scups dataset missing for ...
============ 20 failed, 130 passed, 20 skipped, 2 errors in 18.57s =============
I suppose to test the older versions, these functions would need to be ignored.
Ok that's not too surprising. We could just use the requires_dbase_version
mark to skip these if we're running on a version less than 8. Once we actually support using the information in the splups files to do the needed calculation, we could unskip anything that doesn't specifically test reading the scups files.
Right, I've updated the tests to ignore the ones that require .scups
files for now. I had to modify the fixture to allow multiple conditions (two functions require version > 7 and < 9). I've added a test for version 7 of the database.
The version 9 test is failing now, though, so perhaps I've done the fixture incorrectly? I can't see the root of the issue right now. It runs correctly in the command line...
Ok I think I've fixed it. There was a small bug in handling multiple conditions, but I went ahead and just refactored that mark as my original implementation was clumsy anyway. This also makes using the marks a bit less awkward, i.e. you just specify a string rather than always needing two string which makes handling multiple conditions easier.
This does implicitly assume the conditions are joined by an AND
as opposed to an OR
, but I think that's fine for now.
Thanks for the help! My coding was probably a bit awkward there.
No worries. My initial implementation made it awkward to extend to multiple conditions.
Thanks as always for taking this on!
Fixes #17
(There are a couple of notes in that thread about how to modify functions to use
.splups
files that this does not address. I've only added the file reader here.)Tested with data from version 7.1. I tried both C V and C VI since their
.splups
files have different lengths.