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

Data source fix #841

Merged
merged 6 commits into from May 9, 2016
Merged

Data source fix #841

merged 6 commits into from May 9, 2016

Conversation

FlanFlanagan
Copy link
Contributor

A key error catch for missing cross sections within a cross section cache when building TAPE9.INP files. Probably needs some refinement.

@scopatz scopatz added the bug label Apr 26, 2016
@scopatz scopatz added this to the v0.6 milestone Apr 26, 2016
_compute_xslib(nuc, key, t9[nlb[1]], xscache)
if nuc in FISSION_PRODUCT_NUCS:
_compute_xslib(nuc, key, t9[nlb[2]], xscache)
try:
Copy link
Member

Choose a reason for hiding this comment

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

Try blocks should be a couple of lines at most, and ideally should only be 1 line This is too all-encompassing. I think you should have three try-except blocks, one in each of the conditionals.

@scopatz
Copy link
Member

scopatz commented Apr 26, 2016

Thanks for putting this in @FlanFlanagan!

@scopatz
Copy link
Member

scopatz commented Apr 26, 2016

Also, could you please add a test to ensure that the new KeyError issue happens? This should be pretty easy to do since you can create a cache with no data sources and then assert that it raises a KeyError whenever you try to get anything out of it.

@FlanFlanagan
Copy link
Contributor Author

@scopatz ping. I fixed the test, I think.

@scopatz
Copy link
Member

scopatz commented May 9, 2016

Thanks @FlanFlanagan!

@scopatz scopatz merged commit 3ad4c30 into pyne:develop May 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants