superbit-collaboration / superbit-metacal

Contains a collection of routines used to perform gmix/metacalibration on simulated SuperBIT images
4 stars 1 forks source link

Streamline `SuperBITParameters` IO to be dict based instead of attributes #65

Open sweverett opened 2 years ago

sweverett commented 2 years ago

Right now, SuperBITParameters has to be updated every time we add a potential config option in _load_dict(). This is pretty annoying, and requires a lot of hasattr() checks in various places to see if an attribute has been set as there is no place where they are initially set to None beforehand. It would be a whole lot simpler if we just made a config dictionary to be the attribute that loads in the config options, and then we can do simple checks like if name in sbparams.config.keys():.

Not pressing for initial mock generation, but will make refactoring for real data much simpler.

mcclearyj commented 2 years ago

Definitely agree -- the current galsim config structure is annoying and inelegant.

On Tue, Jun 14, 2022, 1:35 AM Spencer Everett @.***> wrote:

Right now, SuperBITParameters has to be updated every time we add a potential config option in _load_dict(). This is pretty annoying, and requires a lot of hasattr() checks in various places to see if an attribute has been set as there is no place where they are initially set to None beforehand. It would be a whole lot simpler if we just made a config dictionary to be the attribute that loads in the config options, and then we can do simple checks like if name in sbparams.config.keys():.

Not pressing for initial mock generation, but will make refactoring for real data much simpler.

— Reply to this email directly, view it on GitHub https://github.com/superbit-collaboration/superbit-metacal/issues/65, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLXI3XCCYNX622XC7JMSKDVO7AT3ANCNFSM5YV2JLOA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

sweverett commented 1 year ago

Reminder to myself to close this issue once #101 is merged, as we are adding this capability for the new imsim module