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

Transition from atomic-dataset config option to tardis-refdata #775

Merged
merged 10 commits into from
Aug 10, 2017

Conversation

vg3095
Copy link
Contributor

@vg3095 vg3095 commented Aug 9, 2017

This PR replaces atomic-dataset config option with tardis-refdata. Now tardis-refdata should point to Tardis reference folder. Repository Link

vg3095 added 6 commits August 8, 2017 18:58
Now config option "atomic-dataset" is changed to "tardis-refdata". This points to Tardis reference files folder .
Using master branch of tardis-refdata repository
@vg3095
Copy link
Contributor Author

vg3095 commented Aug 9, 2017

@wkerzendorf @yeganer You can review this PR.

@wkerzendorf Can you merge PR#2 in tardis-refdata repository.

Change args.atomic_dataset to args.tardis_refdata
.travis.yml Outdated
@@ -20,28 +20,29 @@ env:
- ASTROPY_USE_SYSTEM_PYTEST=1
- SETUP_CMD='test'
- TEST_MODE='normal'
- ATOM_DATA_URL='https://github.com/tardis-sn/tardis-atomdata/raw/master/kurucz_cd23_chianti_H_He.h5.zip'
- TARDIS_REF_DATA_URL='https://github.com/tardis-sn/tardis-refdata.git'
- TARDIS_REF_VERSION='0.2'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please no version for this.

Copy link
Contributor

@yeganer yeganer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very good. Can you please try to run the integration tests on your machine. I think there might be more changes needed to not break them

pytest.config.getvalue('atomic-dataset')))
assert os.path.exists(self.atom_data_filename), ("{0} atomic datafiles"
def setup(self, atomic_data_fname):
assert os.path.exists(atomic_data_fname), ("{0} atomic datafiles"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the check to the fixture

@vg3095
Copy link
Contributor Author

vg3095 commented Aug 9, 2017

@wkerzendorf @yeganer I have updated this PR .
Integration tests are also running.

@wkerzendorf wkerzendorf merged commit 9d7f2d7 into tardis-sn:master Aug 10, 2017
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.

3 participants