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

Support an "ideal" RMF where MATRIX is a scalar (pyfits backend, fix #862) #863

Merged
merged 2 commits into from
Aug 14, 2020

Conversation

DougBurke
Copy link
Contributor

@DougBurke DougBurke commented Jul 22, 2020

Summary

The AstroPy (pyfits) backend could not handle the case of an ideal RMF when the MATRIX column was specified as a scalar and not a 1-element array.

Details

This is to support issue #862 where a MATRIX column is a scalar, not an array. This is aready supported by Crates, so the Astropy backend needs updating. The change is small (if you can't access the first element of an array then treat it as a scalar), and is an extension to the changes in PRs #358 and #412 which dealt with Swift and ROSAT responses.

A test is added, but rather than adding a file to the Sherpa data corpus, we instead just create the file when needed. It also means that we can easily test the values read in as the data can be made to be easily tested (i.e. a small number of bins).

@DougBurke DougBurke marked this pull request as draft July 22, 2020 16:31
@DougBurke
Copy link
Contributor Author

DougBurke commented Jul 22, 2020

A test case is needed, which is why this is still a WIP.

EDITED TO ADD I've now added the test, do have removed the WIP

@DougBurke DougBurke linked an issue Jul 22, 2020 that may be closed by this pull request
@DougBurke DougBurke changed the title Support an "ideal" RMF where MATRIX is a scalar (pyfits backend) Support an "ideal" RMF where MATRIX is a scalar (pyfits backend, fix #862) Jul 22, 2020
@DougBurke DougBurke marked this pull request as ready for review July 22, 2020 20:18
@DougBurke DougBurke requested a review from dtnguyen2 July 22, 2020 20:26
@DougBurke DougBurke added area:code note: easy review Indicates PR requires simple review type:bug labels Jul 22, 2020
@DougBurke
Copy link
Contributor Author

Gah. Another PR that needs rebasing.

This is to support issue sherpa#862 where a MATRIX column is a scalar,
not an array. This is aready supported by Crates, so the Astropy
backend needs updating.
This test requires readnig a file in. Rather than adding another
test file to the dataset this just creates the file local (in a
temporary file) that gets deleted after the test runs. It makes
it easier to create a "minimal" test case.
@DougBurke
Copy link
Contributor Author

In rebasing this I found that two of the "extra" commits from this PR were no-longer needed (as another PR had converted test_io.py to pytest from SherpaTestCase) so I have dropped them. Which makes this more focussed: a commit to add the change, and one to add the test.

@codecov
Copy link

codecov bot commented Aug 11, 2020

Codecov Report

Merging #863 into master will decrease coverage by 0.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #863      +/-   ##
==========================================
- Coverage   62.98%   62.97%   -0.02%     
==========================================
  Files          70       70              
  Lines       24315    24321       +6     
  Branches     3509     3510       +1     
==========================================
  Hits        15315    15315              
- Misses       7862     7867       +5     
- Partials     1138     1139       +1     
Impacted Files Coverage Δ
sherpa/astro/io/pyfits_backend.py 75.99% <71.42%> (-0.10%) ⬇️
sherpa/astro/sim/pragbayes.py 84.23% <0.00%> (-2.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf66c00...c78f388. Read the comment docs.

@wmclaugh wmclaugh merged commit d90e31e into sherpa:master Aug 14, 2020
@DougBurke DougBurke deleted the issue862 branch August 17, 2020 02:58
@wmclaugh wmclaugh added this to the 4.12.2 milestone Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:code note: easy review Indicates PR requires simple review type:bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

loading RMF fails in backend pyfits
3 participants