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
DM-13686: Saving a particular FrameSet as FITS-WCS causes a segfault #40
Conversation
a segfault when writing a particular FrameSet as FITS-WCS
for unit tests
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.
Two comments, otherwise looks fine.
tests/test_channel.py
Outdated
@@ -6,7 +6,7 @@ | |||
import astshim as ast | |||
from astshim.test import MappingTestCase | |||
|
|||
DataDir = os.path.join(os.path.dirname(__file__)) | |||
DataDir = os.path.join(os.path.dirname(__file__), "data") |
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.
Is this really the standard approach? I thought one would want to put this in setUp
or setUpClass
?
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.
Fair enough. I'll change all three tests that use DataDir
to set self.dataDir in
setUp`
"""Test that a particular FrameSet will not segfault when | ||
we attempt to write it to a FitsChan as FITS-WCS | ||
""" | ||
def readObjectFromShow(path): |
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.
Would this be a useful utility function to lift into astshim itself?
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.
Possibly, but I'm reluctant to expand the API without a clear need. I'd have to think more about the name, for one thing.
The fix is in starlink_ast; this branch adds a unit test