Improve handling for missing required args

taldcroft commented 2 years ago

Description

In the course of investigating the sparkles failure from https://github.com/sot/sparkles/pull/159, I noticed that the handling of missing required arguments could be improved.

This update catches exceptions in a likely place and provides a more useful error message.

Testing

[x] Passes unit tests on MacOS (with new tests)
[x] Functional testing

Functional testing

Ran the new exception cases in IPython and confirmed the expected exception chain for the obsid=99999 case.

jeanconn commented 2 years ago

This looks like a good improvement, but I suppose I'm still not 100% clear on what we want the behavior to be with an obsid + some parameters:

In [21]: get_aca_catalog(obsid=2121, dither_acq=(20, 20), dither_guide=(20,20), date='2021:001')                                                         
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-21-9704d8b7c20d> in <module>
----> 1 get_aca_catalog(obsid=2121, dither_acq=(20, 20), dither_guide=(20,20), date='2021:001')

/proj/sot/ska/jeanproj/git/proseco/proseco/__init__.py in get_aca_catalog(*args, **kwargs)
      6 def get_aca_catalog(*args, **kwargs):
      7     from .catalog import get_aca_catalog
----> 8     return get_aca_catalog(*args, **kwargs)
      9 
     10 

/proj/sot/ska/jeanproj/git/proseco/proseco/catalog.py in get_aca_catalog(obsid, **kwargs)
    127             aca = _get_aca_catalog_monitors(obsid=obsid, raise_exc=raise_exc, **kwargs)
    128         else:
--> 129             aca = _get_aca_catalog(obsid=obsid, raise_exc=raise_exc, **kwargs)
    130 
    131     except Exception:

/proj/sot/ska/jeanproj/git/proseco/proseco/catalog.py in _get_aca_catalog(**kwargs)
    149 
    150     aca = ACATable()
--> 151     aca.set_attrs_from_kwargs(**kwargs)
    152     aca.call_args = kwargs.copy()
    153     aca.version = VERSION

/proj/sot/ska/jeanproj/git/proseco/proseco/catalog.py in set_attrs_from_kwargs(self, **kwargs)
    395         if self.t_ccd_penalty_limit is None:
    396             self.t_ccd_penalty_limit = ACA.aca_t_ccd_penalty_limit
--> 397         self.t_ccd_eff_acq = get_effective_t_ccd(self.t_ccd_acq, self.t_ccd_penalty_limit)
    398         self.t_ccd_eff_guide = get_effective_t_ccd(self.t_ccd_guide, self.t_ccd_penalty_limit)
    399         self.version = VERSION

/proj/sot/ska/jeanproj/git/proseco/proseco/catalog.py in get_effective_t_ccd(t_ccd, t_ccd_penalty_limit)
    318     t_limit = (ACA.aca_t_ccd_penalty_limit if t_ccd_penalty_limit is None
    319                else t_ccd_penalty_limit)
--> 320     if t_ccd > t_limit:
    321         return t_ccd + 1 + (t_ccd - t_limit)
    322     else:

TypeError: '>' not supported between instances of 'NoneType' and 'float'

taldcroft commented 2 years ago

@jeanconn - awesome catch! The root cause is that the required_attrs was not set correctly for ACATable.

sot / proseco