Ticket #396 (closed defect: fixed)

Opened 11 years ago

Last modified 7 years ago

Getters in LsstBase should return None in Python if the associated data is not set

Reported by: rowen Owned by: ktl
Priority: normal Milestone:
Component: daf_data Keywords:
Cc: smm Blocked By:
Blocking: Project: LSST
Version Number: 3.0
How to repeat:

not applicable

Description

Right now getters in LsstBase? return a null pointer if the associated data has never been set. Attempting to use the returned object in python causes an abort (exit from the interpreter) as per ticket #389. This is not safe. If possible, please have the python code return None. If that is not practical then a reasonable second best is to return an empty container.

Change History

comment:1 Changed 11 years ago by ktl

  • Status changed from new to assigned

comment:2 Changed 11 years ago by ktl

  • Status changed from assigned to inTicketWork

comment:3 Changed 11 years ago by ktl

  • Owner changed from ktl to rowen
  • Status changed from inTicketWork to needinfo

Please try daf/data/tickets/396 [6110] to see if that has the intended effect.

comment:4 Changed 11 years ago by rowen

I am still seeing the problem. What I did:

svn co $LSST_SVN/DMS/daf/data/tickets/396 daf_data396
cd daf_data396
eups declare -r $PWD daf_data 396
setup daf_data 396
scons
cd ../afw_trunk
setup afw --keep
eups list daf_data -v
   396          /Users/rowen/lsst_root  /Users/rowen/LSST/code/daf_data396               Setup
   999          /Users/rowen/lsst_root  /Users/rowen/LSST/code/daf_data-trunk            Current
eups list afw -v
   999          /Users/rowen/lsst_root  /Users/rowen/LSST/code/afw-trunk                 Current Setup
scons --clean
scons opt=3

I then ran the following python script:

import os
import lsst.afw.image as afwImage
dataDir = os.environ["AFWDATA_DIR"]
im = afwImage.ImageF()
im.readFits(os.path.join(dataDir, "small_MI_img.fits"))
badMetadata = im.getMetadata()
# any of the following will abort:
badMetadata.isNode()
badMetadata.getChildren()

Either of the final statements caused it to abort. Replacing "getMetadata" with "getMetaData" made the script run fine.

comment:5 Changed 11 years ago by rowen

  • Owner changed from rowen to ktl
  • Status changed from needinfo to assigned

comment:6 Changed 11 years ago by ktl

  • Owner changed from ktl to rowen
  • Status changed from assigned to needinfo

Umm... I think you need to re-scons daf_data first, before rebuilding afw.

comment:7 Changed 11 years ago by rowen

  • Owner changed from rowen to ktl
  • Status changed from needinfo to assigned

I don't understand. Of course I sconsed daf_data; I had to -- nothing would have worked if I had forgotten because I checked out source from ticket 396. Or was that a typo and you meant to say that I should rebuild some other package that depends on daf_data (other than afw)?

comment:8 Changed 11 years ago by ktl

  • Status changed from assigned to inTicketWork

Oops -- I missed that line. Very sorry.

The problem is that the fix in daf_data's dataLib.i does not propagate to afw, since SWIG never sees it. I'm trying something else...

comment:9 Changed 11 years ago by ktl

OK, I've managed to get this to work properly for an ImageFPtr by changing afw/image/imageLib.i, but I'm still getting a couple of SWIG warnings, and it still doesn't work for an ImageF, so this will take some more SWIG hackery...

comment:10 Changed 11 years ago by ktl

  • Cc smm added
  • Owner changed from ktl to rowen
  • Status changed from inTicketWork to needinfo

afw/tickets/396 [6124] should fix this, in combination with daf/data/tickets/396, for ImageF. Please check that this works. I believe that [6124] is part of what Serge is planning to do for the general SWIG fixes.

comment:11 Changed 11 years ago by rowen

That seems to have fixed it, but by hiding the getMetadata method. Is that what you intended? Does this mean that every user of LsstBase? will have to manually hide the inappropriate methods?

comment:12 Changed 11 years ago by rowen

  • Owner changed from rowen to ktl
  • Status changed from needinfo to assigned

comment:13 Changed 11 years ago by ktl

getMetadata() for ImageF should not be hidden. It is available through the LsstBase and then LsstImpl_DC3 base classes. There, it is a custom Python function that wraps the built-in SWIG wrapper call. All users of LsstBase will need to %include "../python/lsst/daf/data/dataLib.i", but I believe this is going to be the accepted method for doing such inclusions in the future.

comment:14 Changed 11 years ago by rowen

The getMetadata method appears to be gone. Here is what I see:

lsst
setup daf_data 396
setup afw 396 --keep
Not setting up daf_data as it is already setup locally
eups list daf_data -v
   396          /Users/rowen/lsst_root  /Users/rowen/LSST/code/daf_data396               Setup
   999          /Users/rowen/lsst_root  /Users/rowen/LSST/code/daf_data-trunk            Current
eups list afw -v
   396          /Users/rowen/lsst_root  /Users/rowen/LSST/code/afw396            Setup
   999          /Users/rowen/lsst_root  /Users/rowen/LSST/code/afw-trunk                 Current

Then run this python script:

import os
import lsst.afw.image as afwImage
dataDir = os.environ["AFWDATA_DIR"]
im = afwImage.ImageF()
im.readFits(os.path.join(dataDir, "small_MI_img.fits"))
badMetadata = im.getMetadata()

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/rowen/LSST/code/afw396/python/lsst/afw/image/imageLib.py", line 2558, in <lambda>
    __getattr__ = lambda self, name: _swig_getattr(self, ImageF, name)
  File "/Users/rowen/LSST/code/afw396/python/lsst/afw/image/imageLib.py", line 41, in _swig_getattr
    raise AttributeError,name
AttributeError: getMetadata

comment:15 Changed 11 years ago by ktl

Oops again. Forgot to check in the companion change to daf_data, [6125] in tickets/396. That ought to do it.

comment:16 Changed 11 years ago by rowen

That worked! getMetadata returned None, as desired.

comment:17 Changed 11 years ago by ktl

  • Status changed from assigned to closed
  • Resolution set to fixed

comment:18 Changed 11 years ago by ktl

[6136] and [6140] merge the minimal fixes for this ticket to the trunk in daf_data and afw, respectively.

comment:19 Changed 7 years ago by robyn

  • Milestone DC3 Design deleted

Milestone DC3 Design deleted

Note: See TracTickets for help on using tickets.