-
-
Notifications
You must be signed in to change notification settings - Fork 435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test and fix for issue #1228 #1235
Merged
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. |
Co-authored-by: Alice Harpole <[email protected]>
Co-authored-by: Alice Harpole <[email protected]>
Co-authored-by: Alice Harpole <[email protected]>
add Christian's corrections Co-authored-by: Christian Vogl <[email protected]>
wkerzendorf
reviewed
Aug 7, 2020
@@ -0,0 +1,4 @@ | |||
Index H He Ni56 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe give this a check
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 <[email protected]> * Update tardis/model/tests/test_csvy_model.py Co-authored-by: Alice Harpole <[email protected]> * Update tardis/model/tests/test_csvy_model.py Co-authored-by: Alice Harpole <[email protected]> * Alice's corrections Co-authored-by: Alice Harpole <[email protected]> * 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 <[email protected]> * 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 <[email protected]> * Update tardis/model/tests/test_csvy_model.py Co-authored-by: Alice Harpole <[email protected]> * Alice's corrections Co-authored-by: Alice Harpole <[email protected]> * 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 <[email protected]> * move reference data to fixtures * small clarification in file * delete duplicate tests * move docstring to fixture * fix issue-1228 Co-authored-by: Alice Harpole <[email protected]> Co-authored-by: Christian Vogl <[email protected]>
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.dat
density.dat
→custom_density_tardis.dat
config_csvy_nocsv_branch85_old.yml
→branch85_old_config.yml
config_csvy_nocsv_branch85.yml
→branch85_csvy.yml
csvy_nocsv.csvy
→branch85_csvy.csvy
config_csvy_nocsv_exponential_old.yml
→exponential_old_config.yml
config_csvy_nocsv_exponential.yml
→exponential_csvy.yml
csvy_nocsv_exponential.csvy
→exponential_csvy.csvy
config_csvy_full_old.yml
→model_full_old_config.yml
config_csvy_full.yml
→model_full_csvy.yml
csvy_full.csvy
→model_full_csvy.csvy
config_csvy_nocsv_powerlaw_old.yml
→powerlaw_old_config.yml
config_csvy_nocsv_powerlaw.yml
→powerlaw_csvy.yml
csvy_nocsv_powerlaw.csvy
→powerlaw_csvy.csvy
config_csvy_full_rad_old.yml
→radiative_old_config.yml
config_csvy_full_rad.yml
→radiative_csvy.yml
csvy_full_rad.csvy
→radiative_csvy.csvy
config_csvy_nocsv_uniform_old.yml
→uniform_old_config.yml
config_csvy_nocsv_uniform.yml
→uniform_csvy.yml
csvy_nocsv_uniform.csvy
→uniform_csvy.csvy
config_v_filter.yml
→csvy_model_to_test_abundances.yml
csvy_vfilter.csvy
→csvy_model_to_test_abundances.csvy
Unused files where removed:
csvy_isotope_time.csvy
csvy_full_rad_abundance_test.csvy
config_csvy_abund_test.yml
csvy_isotope_time.csvy
Motivation and Context
See issue #1228 for a full explanation. They appear as new asserts because
get_properties
doesn't takeabundance
as 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?
pytest
fails as expectedTypes of changes
Checklist: