c3-time-domain / SeeChange

A time-domain data reduction pipeline (e.g., for handling images->lightcurves) for surveys like DECam and LS4
BSD 3-Clause "New" or "Revised" License
0 stars 4 forks source link

String parameters often accepted with mixed case #197

Closed guynir42 closed 1 month ago

guynir42 commented 5 months ago

I have been generous and gave the user, in many places, the option of spelling out string-based parameters as case insensitive. E.g., you can ask for a subtraction method ZOGY or Zogy or zogy and they all get .lower() before being checked against the possible values for that parameter.

This is great for the users, but notice that ZOGY and zogy will end up with different Provenances.

On the one hand, this is not as bad as having truly different values ending up with the same provenance. On the other hand, this allows the careless user (and who isn't?) to end up with two provenances for something that really shouldn't.

Should we just stupid-proof this by requiring exact string matches everywhere, and enforce that configurations as for exactly the right string?

Also, what should the policy on capitalization be? Should we demand all strings to be lower case only? Some acronyms are more familiar as all-caps but I think we can agree that psf and zogy are just as good as PSF and ZOGY.

rknop commented 3 months ago

Proposed solution : standardize on all lowercase for all parameters.

Right now, the only one we found (with a quick search) is a GAIA catalog reference. (See .yaml file.)

guynir42 commented 3 months ago

Agreed: we want lower case parameters only, and use underscore to separate more than one word...

guynir42 commented 3 months ago

Addressed by #231