feder-observatory / stellarphot

Stellar aperture photometry
https://stellarphot.readthedocs.io
BSD 3-Clause "New" or "Revised" License
4 stars 12 forks source link

Add `fwhm` property to `ApertureSettings`? #254

Closed mwcraig closed 9 months ago

mwcraig commented 9 months ago

I've been looking at how to condense the arguments to the photometry routines and was thinking about adding a FWHM property to ApertureSettings. I wanted to get some feedback before doing it though.

In favor of adding FWHM:

Against adding FWHM:

Thoughts, @JuanCab?

JuanCab commented 9 months ago

OK, this is me being difficult, but it would be easier if it was accurately called 'AperturePhotometrySettings' or 'ApPhotoSettings', because in my mind the assumed FWHM IS a setting related to aperture photometry, and that is what we call ApertureSettings really is. Sure, it describes the aperture, but only for use in performing aperture photometry. Does that make sense?

So I would be in favor of adding it and if you are particularly masochistic, changing the name of the settings to better reflect their purpose. :)

mwcraig commented 9 months ago

So I would be in favor of adding it and if you are particularly masochistic, changing the name of the settings to better reflect their purpose. :)

I think that vs code makes this pretty easy: right clicking on ApertureSettings and choosing Rename symbol... gets 99% of the changes done. Not sure about things like docstrings.

On the upside, up nothing to change in the documentation 😬

mwcraig commented 9 months ago

How do you feel about the name PhotometryApertures? I'm thinking about something like PhotometrySettings for the larger constellation of settings to the photometry functions.

JuanCab commented 9 months ago

PhotometryApertures is certainly better than ApertureSettings at conveying where the settings will be used.