test and fix for issue #1228 #1235
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 tasks
6 tasks
marxwillia
reviewed
Jul 20, 2020
marxwillia
reviewed
Jul 20, 2020
marxwillia
reviewed
Jul 20, 2020
marxwillia
reviewed
Jul 20, 2020
marxwillia
reviewed
Jul 20, 2020
marxwillia
reviewed
Jul 20, 2020
marxwillia
reviewed
Jul 20, 2020
marxwillia
reviewed
Jul 20, 2020
marxwillia
reviewed
Jul 20, 2020
marxwillia
reviewed
Jul 20, 2020
|
Let's change how the pytest filename fixture works to be more descriptive, and more concise. |
ebjordi
commented
Jul 20, 2020
| @@ -42,16 +42,48 @@ def test_compare_models(filename): | |||
| config_model_val = config_model_val.value | |||
| csvy_model_val = csvy_model_val.value | |||
| npt.assert_array_almost_equal(csvy_model_val, config_model_val) | |||
| pd.testing.assert_frame_equal(csvy_model.abundance,config_model.abundance,check_less_precise = True) | |||
There was a problem hiding this comment.
This are the lines that check that the models' abundances are equal. There was an issue were this dataframes were not compared and both implementations had different outputs.
This test both values and shapes of dataframes.
Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com>
Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com>
Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com>
add Christian's corrections Co-authored-by: Christian Vogl <cvogl@mpa-garching.mpg.de>
wkerzendorf
reviewed
Aug 7, 2020
| @@ -0,0 +1,4 @@ | |||
| Index H He Ni56 | |||
harpolea
approved these changes
Aug 10, 2020
atharva-2001
pushed a commit
to atharva-2001/tardis
that referenced
this pull request
Oct 1, 2021
* test for detecting error * files needed for tests * add test for csvy vs. hard coded data * add test fir config vs. hard coded data * make tests and test files more descriptive * remove condig model test and asociated files, remove unused files * make filenames more descriptive * remove more unused files * rename files for other test * fix filename join * fix errors in tests * Apply black and add descriptions to tests * make descriptions more uniform across files * restore line accidentaly removed * More docstrings and clarifications and PEP8 * rename files to make their purpose clearer * fix wrong extension for csvy file * change wording of comment * change abundance and density files names and changed references * restore base.py accidentally committed before * fix wording in docstring * Update tardis/model/tests/test_csvy_model.py Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com> * Update tardis/model/tests/test_csvy_model.py Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com> * Update tardis/model/tests/test_csvy_model.py Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com> * Alice's corrections Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com> * change wording of docstring * change filenames function name * fix typos, clarifications * change name to reference abundance and spellcheck docstring * Apply suggestions from code review add Christian's corrections Co-authored-by: Christian Vogl <cvogl@mpa-garching.mpg.de> * move reference data to fixtures * small clarification in file * delete duplicate tests * test for detecting error * files needed for tests * add test for csvy vs. hard coded data * add test fir config vs. hard coded data * make tests and test files more descriptive * remove condig model test and asociated files, remove unused files * make filenames more descriptive * remove more unused files * rename files for other test * fix filename join * fix errors in tests * Apply black and add descriptions to tests * make descriptions more uniform across files * restore line accidentaly removed * More docstrings and clarifications and PEP8 * rename files to make their purpose clearer * fix wrong extension for csvy file * change wording of comment * change abundance and density files names and changed references * restore base.py accidentally committed before * fix wording in docstring * change wording of docstring * Update tardis/model/tests/test_csvy_model.py Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com> * Update tardis/model/tests/test_csvy_model.py Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com> * Alice's corrections Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com> * change filenames function name * fix typos, clarifications * change name to reference abundance and spellcheck docstring * Apply suggestions from code review add Christian's corrections Co-authored-by: Christian Vogl <cvogl@mpa-garching.mpg.de> * move reference data to fixtures * small clarification in file * delete duplicate tests * move docstring to fixture * fix issue-1228 Co-authored-by: Alice Harpole <harpolea@users.noreply.github.com> Co-authored-by: Christian Vogl <cvogl@mpa-garching.mpg.de>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added tests and rename files to test abundances from TARDIS model.
Tests renamed, modified, and with added docstring:
filename→model_config_files: renamed and added docstring.test_compare_models: added assert statements to test abundances data frames betweenmodels.
test_csvy_abundance→test_read_csvy_abundances: add comparison of shapes of abundance data frame and added test for isotope abundance (both values and shapes).New tests:
csvy_model_to_test_abundances: construct csvy model used by some tests.test_csvy_model_decay: Compare model abundance decay against hand made decay calculations.data/files renamed and with added descriptions:abun.dat→custom_abundance_tardis.datdensity.dat→custom_density_tardis.datconfig_csvy_nocsv_branch85_old.yml→branch85_old_config.ymlconfig_csvy_nocsv_branch85.yml→branch85_csvy.ymlcsvy_nocsv.csvy→branch85_csvy.csvyconfig_csvy_nocsv_exponential_old.yml→exponential_old_config.ymlconfig_csvy_nocsv_exponential.yml→exponential_csvy.ymlcsvy_nocsv_exponential.csvy→exponential_csvy.csvyconfig_csvy_full_old.yml→model_full_old_config.ymlconfig_csvy_full.yml→model_full_csvy.ymlcsvy_full.csvy→model_full_csvy.csvyconfig_csvy_nocsv_powerlaw_old.yml→powerlaw_old_config.ymlconfig_csvy_nocsv_powerlaw.yml→powerlaw_csvy.ymlcsvy_nocsv_powerlaw.csvy→powerlaw_csvy.csvyconfig_csvy_full_rad_old.yml→radiative_old_config.ymlconfig_csvy_full_rad.yml→radiative_csvy.ymlcsvy_full_rad.csvy→radiative_csvy.csvyconfig_csvy_nocsv_uniform_old.yml→uniform_old_config.ymlconfig_csvy_nocsv_uniform.yml→uniform_csvy.ymlcsvy_nocsv_uniform.csvy→uniform_csvy.csvyconfig_v_filter.yml→csvy_model_to_test_abundances.ymlcsvy_vfilter.csvy→csvy_model_to_test_abundances.csvyUnused files where removed:
csvy_isotope_time.csvycsvy_full_rad_abundance_test.csvyconfig_csvy_abund_test.ymlcsvy_isotope_time.csvyMotivation and Context
See issue #1228 for a full explanation. They appear as new asserts because
get_propertiesdoesn't takeabundanceas such and therefore it is not comparedWhy wasn't it detected before?
There are no tests that check that the isotope abundance and final abundances are what we expect.
How Has This Been Tested?
pytestfails as expectedTypes of changes
Checklist: