Skip to content
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 72 commits into from
Aug 10, 2020
Merged

test and fix for issue #1228 #1235

merged 72 commits into from
Aug 10, 2020

Conversation

ebjordi
Copy link
Contributor

@ebjordi ebjordi commented Jul 14, 2020

Description

Added tests and rename files to test abundances from TARDIS model.

Tests renamed, modified, and with added docstring:

  • filenamemodel_config_files : renamed and added docstring.
  • test_compare_models : added assert statements to test abundances data frames between
    models.
  • test_csvy_abundancetest_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.datcustom_abundance_tardis.dat
  • density.datcustom_density_tardis.dat
  • config_csvy_nocsv_branch85_old.ymlbranch85_old_config.yml
  • config_csvy_nocsv_branch85.ymlbranch85_csvy.yml
  • csvy_nocsv.csvybranch85_csvy.csvy
  • config_csvy_nocsv_exponential_old.ymlexponential_old_config.yml
  • config_csvy_nocsv_exponential.ymlexponential_csvy.yml
  • csvy_nocsv_exponential.csvyexponential_csvy.csvy
  • config_csvy_full_old.ymlmodel_full_old_config.yml
  • config_csvy_full.ymlmodel_full_csvy.yml
  • csvy_full.csvymodel_full_csvy.csvy
  • config_csvy_nocsv_powerlaw_old.ymlpowerlaw_old_config.yml
  • config_csvy_nocsv_powerlaw.ymlpowerlaw_csvy.yml
  • csvy_nocsv_powerlaw.csvypowerlaw_csvy.csvy
  • config_csvy_full_rad_old.ymlradiative_old_config.yml
  • config_csvy_full_rad.ymlradiative_csvy.yml
  • csvy_full_rad.csvyradiative_csvy.csvy
  • config_csvy_nocsv_uniform_old.ymluniform_old_config.yml
  • config_csvy_nocsv_uniform.ymluniform_csvy.yml
  • csvy_nocsv_uniform.csvyuniform_csvy.csvy
  • config_v_filter.ymlcsvy_model_to_test_abundances.yml
  • csvy_vfilter.csvycsvy_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 take abundance as such and therefore it is not compared

Why 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 expected

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have assigned/requested two reviewers for this pull request.

@marxwillia
Copy link
Contributor

Let's change how the pytest filename fixture works to be more descriptive, and more concise.

@@ -0,0 +1,4 @@
Index H He Ni56
Copy link
Member

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

@ebjordi ebjordi changed the title test for detecting issue #1228 test and fix for issue #1228 Aug 7, 2020
@wkerzendorf wkerzendorf merged commit d9bb2f1 into tardis-sn:master 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants