[DAMD-61] Writing FITS to a text-mode file is not permitted in astropy 3.2 Created: 13/Jun/19  Updated: 05/Jul/19  Resolved: 05/Jul/19

Status: Done
Project: Data Model
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Normal
Reporter: sogo.mineo Assignee: sogo.mineo
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Sprint: 2DDRP-2019 F

 Description   

Astropy 3.2 requires the following fix to pfsSpectra.py.

datamodel/python/pfs/datamodel/pfsSpectra.py:208:

-with open(filename, "w") as fd:
+with open(filename, "wb") as fd:
    fits.writeto(fd)

Otherwise, the following exception is raised.

  File ".../pfsrepos/datamodel/python/pfs/datamodel/pfsSpectra.py", line 209, in writeFits
    fits.writeto(fd)
  File ".../lib/python3.6/site-packages/astropy/utils/decorators.py", line 521, in wrapper
    return function(*args, **kwargs)
  File ".../lib/python3.6/site-packages/astropy/io/fits/hdu/hdulist.py", line 915, in writeto
    mode = FILE_MODES[fileobj_mode(fileobj)] if isfile(fileobj) else 'ostream'
KeyError: 'w'


 Comments   
Comment by sogo.mineo [ 13/Jun/19 ]

Here's a suggested patch.

diff --git a/python/pfs/datamodel/pfsConfig.py b/python/pfs/datamodel/pfsConfig.py
index 3684080..abd78e5 100644
--- a/python/pfs/datamodel/pfsConfig.py
+++ b/python/pfs/datamodel/pfsConfig.py
@@ -252,7 +252,7 @@ class PfsDesign:
         ], hdr, name='PHOTOMETRY'))

         # clobber=True in writeto prints a message, so use open instead
-        with open(filename, "w") as fd:
+        with open(filename, "wb") as fd:
             fits.writeto(fd)

     def write(self, dirName=".", fileName=None):
diff --git a/python/pfs/datamodel/pfsSpectra.py b/python/pfs/datamodel/pfsSpectra.py
index a150e0e..f66f2d8 100644
--- a/python/pfs/datamodel/pfsSpectra.py
+++ b/python/pfs/datamodel/pfsSpectra.py
@@ -205,7 +205,7 @@ class PfsSpectra:
             hduName = attr.upper()
             data = getattr(self, attr)
             fits.append(astropy.io.fits.ImageHDU(data, name=hduName))
-        with open(filename, "w") as fd:
+        with open(filename, "wb") as fd:
             fits.writeto(fd)

     def write(self, dirName="."):
diff --git a/python/pfs/datamodel/pfsSpectrum.py b/python/pfs/datamodel/pfsSpectrum.py
index 5c40ff1..c4026a6 100644
--- a/python/pfs/datamodel/pfsSpectrum.py
+++ b/python/pfs/datamodel/pfsSpectrum.py
@@ -165,7 +165,7 @@ class PfsSimpleSpectrum:
         fits = HDUList()
         fits.append(PrimaryHDU())
         self._writeImpl(fits)
-        with open(filename, "w") as fd:
+        with open(filename, "wb") as fd:
             fits.writeto(fd)

     def write(self, dirName="."):
Comment by hassan [ 14/Jun/19 ]

sogo.mineo: re-assigned to you following the recent discussions on the slack #drp-2d channel. Please apply your patch to the ticket branch, run the integration test and create a pull request as per the standard procedure.

Comment by sogo.mineo [ 14/Jun/19 ]

I created a pull request just now.

Comment by hassan [ 01/Jul/19 ]

sogo.mineo: the pull request has been approved so you can merge to master.

Comment by sogo.mineo [ 02/Jul/19 ]

I thought that a maintainer would merge and push to the master instead of me. Do I have permission to do it?

Comment by hassan [ 02/Jul/19 ]

Yes, once the pull request has been approved the developer has permission to merge to master and close the ticket. If you do not have formal access to update git or JIRA this way, please contact me or jira@pfs.ipmu.jp . In the meantime, I will merge and close this ticket on your behalf.

Comment by sogo.mineo [ 04/Jul/19 ]
  • In addition to datamodel's tickets/DAMD-61, I have pushed another branch tickets/DAMD-61 to drp_stella. Do I have to make a pull request for this one?
  • The integration test ends by saying
    nan
    Done.
    

    Can I ignore the printed value "nan" and believe the last word "Done" to go ahead?

Comment by hassan [ 04/Jul/19 ]

Yes - please create another pull request for the drp_stella changes.

The output of the integration test as you describe is correct, so once the PR on drp_stella is approved, you can proceed with the merging to master.

Comment by hassan [ 04/Jul/19 ]

Pull request #73 also accepted. Please merge to master and close ticket.

Comment by sogo.mineo [ 05/Jul/19 ]

hassan Thank you. I close this ticket

Generated at Sat Feb 10 15:33:50 JST 2024 using Jira 8.3.4#803005-sha1:1f96e09b3c60279a408a2ae47be3c745f571388b.