planetarypy / planetaryimage

Python PDS and Isis Cube file parser.
BSD 3-Clause "New" or "Revised" License
40 stars 20 forks source link

Improve Error when opening Detached Label #75

Open percurnicus opened 7 years ago

percurnicus commented 7 years ago

There should be a better error when attempting to open a detached label image with the image file rather than the label file. This is the most common error I come across:

    121         if isinstance(key, INDEX_TYPES):
    122             return self.__items[key]
--> 123         return dict_getitem(self, key)[0]
    124 
    125     def __delitem__(self, key):

KeyError: '^IMAGE'

Which doesn't really explain to me that there is a detached label I should open. I don't have any immediate ideas on how to make this more explicit. In a perfect world, we would search for the label file ourselves but since there is a lack of consistency in the extensions, I don't see how my perfect world can exist.

thareUSGS commented 7 years ago

I'm not sure I understand since you should never need to guess an extension. The full filename should be listed. Here are some valid ^Image uses.

 ^IMAGE = 3
 ^IMAGE             = "GLOBAL_ALBEDO_8PPD.IMG"
 ^IMAGE             = "MEGT90N000CB.IMG"
 ^IMAGE             = ("BLAH.IMG",1)       -- start at record 1 (1 based)
 ^IMAGE             = ("BLAH.IMG")         -- still start at record 1 (equiv of "BLAH.IMG")
 ^IMAGE             = ("BLAH.IMG", 5 <BYTES>) -- start at byte 5 (the fifth byte in the file)
 ^IMAGE             = 10851 <BYTES>
 ^SPECTRAL_QUBE = 5  -- for multi-band images

you can see how GDAL attempts to handle this but below it gets more complicated; https://svn.osgeo.org/gdal/trunk/gdal/frmts/pds/pdsdataset.cpp


Another label complication, which I really didn't like see incorporated, was the allowed additions for COMPRESSED_FILE and UNCOMPRESSED_FILE in the same label. The problem being now the code has to make a choice. Do I open the compressed file or uncompressed file? They should be the same but... Some label examples for this.

OBJECT = COMPRESSED_FILE
  FILE_NAME = "FILENAME.JP2"
  RECORD_TYPE = UNDEFINED
  ENCODING_TYPE = "JP2"
...

OBJECT = UNCOMPRESSED_FILE
   FILE_NAME = "FILENAME.IMG"
   RECORD_TYPE = FIXED_LENGTH
 ...

And ^IMAGE can also exist in the UNCOMPRESSED_FILE section too. some more on this: https://pds.jpl.nasa.gov/documents/sr/AppendixI.pdf

percurnicus commented 7 years ago

Sorry, I meant to include an example.

>>> from planetaryimage import PDS3Image
>>> image = PDS3Image('tests/mission_data/0047MH0000110010100214C00_DRCL.IMG')
...
    121         if isinstance(key, INDEX_TYPES):
    122             return self.__items[key]
--> 123         return dict_getitem(self, key)[0]
    124 
    125     def __delitem__(self, key):

KeyError: '^IMAGE'
>>> image = PDS3Image('tests/mission_data/0047MH0000110010100214C00_DRCL.LBL')
>>> image.bands
3

What I don't like is the error is not explicit that the .IMG file cannot be opened because it has a detached label. Furthermore, opening the label in order to open the image just feels odd. Maybe that is due to the fact I have only been dealing with PDS for a couple years but I do think it is still very implicit. Although it's not unreasonable to expect the user to know if they are working with detached labels, I think we should be more explicit when the user uses the wrong file. Something like this would be better:

>>> from planetaryimage import PDS3Image
>>> image = PDS3Image('tests/mission_data/0047MH0000110010100214C00_DRCL.IMG')
...
    121         if isinstance(key, INDEX_TYPES):
    122             return self.__items[key]
--> 123         return dict_getitem(self, key)[0]
    124 
    125     def __delitem__(self, key):
    Unable to open image. Potentially because the image has a detached label. 
    Try opening the image with the file containing the label.

    Original Error:

    KeyError: '^IMAGE'

As for the uncompressed/compressed sections, that does present a complication, especially with detached labels... would being explicit in the function call be better? This is my initial thought:

>>> from planetaryimage import PDS3Image
>>> # Opening compressed image:
>>> compressed_image = PDS3Image('filename.jp2', lbl_path='filename.lbl')
>>> # opening uncompressed image:
>>> uncompressed_image = PDS3Image('filename.img', lbl_path='filename.lbl')

So we strip as much decision making from the code and put that responsibility on the user. By default the code can use the first instance of ^Image to get the image name (which is currently the case).

thareUSGS commented 7 years ago

That error looks fine to me. You could try to find the .lbl or .LBL if the *.IMG is simply a raw binary stream, but users generally figure they need to point to a label when available.

It is handled now, but GDAL is pretty strict with the PDS hierarchical structure of the PVL label. The original implementation only looked for "^IMAGE" at the root level. When "^IMAGE" started showing up inside the "OBJECT = UNCOMPRESSED_FILE" the group, it would fail to load the image. Nothing to worry about here - just something we had to fix as the standard changed.

percurnicus commented 7 years ago

I thought about looking for .lbl/.LBL but there are other extensions like .haf that would not work. I'm very tempted to still look for the .lbl/.LBL and other extensions have to be explicitly passed in, but on the fence.

thareUSGS commented 7 years ago

What the heck is a "haf" file - never seen that... For an initial push I would just send the error. If you get a lot of feedback to auto-search for other files, then it can be added. The GDAL driver, since it supports so many image formats, can't really be expected to look around for detached labels. But since you are mostly supporting PDS3, perhaps looking for labels could be added.

I think PDS4 files will more likely have detached labels (if not mandatory), so users will be force to get use to detached labels.

BTW, here is the fairly cryptic GDAL error. But really, what else can it report? At least you can warn for a detached label.

>gdalinfo ldec_4.img
ERROR 4: `ldec_4.img' not recognized as a supported file format.
gdalinfo failed - unable to open 'ldec_4.img'.