wtbarnes / fiasco

Python interface to the CHIANTI atomic database
http://fiasco.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
20 stars 15 forks source link

hasattr not working with missing dielectronic properties #268

Closed jwreep closed 3 months ago

jwreep commented 4 months ago

To test whether an ion has dielectronic data, I would think that we could write, e.g.:

carbon = fiasco.Element('C', temperature=1e5*u.K)
print( hasattr(carbon[5], '_dielectronic_elvlc') )

which should in principle return False since there is no dielectronic data. It does correctly return True when the data does exist. However, when it doesn't exist, it returns a KeyError because fiasco is still attempting to access the non-existent data:

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[37], line 1
----> 1 hasattr(carbon[5], '_dielectronic_elvlc')

File ~/Documents/Forks/fiasco/fiasco/base.py:157, in add_property.<locals>.property_template(self)
    155 def property_template(self):
    156     data_path = '/'.join([self.atomic_symbol.lower(), self._ion_name, filetype])
--> 157     return DataIndexer.create_indexer(self.hdf5_dbase_root, data_path)

File ~/Documents/Forks/fiasco/fiasco/io/datalayer.py:32, in DataIndexer.create_indexer(cls, *args)
     30 @classmethod
     31 def create_indexer(cls, *args):
---> 32     return DataIndexerHDF5.create_indexer(*args)

File ~/Documents/Forks/fiasco/fiasco/io/datalayer.py:71, in DataIndexerHDF5.create_indexer(cls, hdf5_path, top_level_path)
     69     return cls(hdf5_path, top_level_path)
     70 else:
---> 71     raise KeyError(f'{top_level_path} not found in {hdf5_path}')

KeyError: 'c/c_6/dielectronic/elvlc not found in /Users/reep/.fiasco/chianti_dbase.h5'

Presumably, it shouldn't try to access the data here! Not sure what the fix is off the top of my head, but I wouldn't think this is the desired behavior.

wtbarnes commented 4 months ago

This is once again a consequence of my poor design choices! The exception occurs because the attribute does indeed exist but there's no data to be accessed, hence the KeyError.

I think a good solution here would be to add a private method to the Ion class that allows you to check whether the needed data exists for that ion. This could come in handy in a few other places as well.

Here's an example of how you might implement this using the needs_dataset decorator:

def _has_dataset(self, dset_name):
    try:
        needs_dataset(dset_name)(lambda _: None)(self)
    except MissingDatasetException:
        return False
    else:
        return True

I wrote this on mobile without testing anything but I think that should work.

wtbarnes commented 4 months ago

I should add, the reason for not just using hasattr for all this is we want the attributes to exist even if the data doesn't since different ions have different data attached to them and the types of data attached to a given ion may change from one database version to the next.