I'll also open this as a place for comments that aren't really relevant to the submission criteria but which I nevertheless hope to be constructive criticism more in line with a general code review.
As a general remark, be aware that most code will be read more often than it is written and reducing "mental load" you place on your audience (which includes future you) is a big factor to the time and nerves it will cost them to understand what is going on. I'd wager a guess and say you and others are also a lot more likely to contribute if they feel "at home". There's a few things that help with that
Stick to established style conventions (pep8) and explain your reasoning when deviating
I tend to make a pep8-exception when it comes to line-length for more verbose variable names (e.g. size->image_size, pos->host_galaxy_coordinates). But keep in mind that at 4-5 layers of indentation, things become really hard to follow, so still set a reasonable limit like 120 chars.
Aim to reduce the number of surprises in general: The same or very similar parameters should have the same naming scheme and type when used in multiple functions.
The number of things one can simultaneously keep in their head is limited, so keep the number of function-parameters manageable. If possible, bundle them together into logical groups, i.e. use a singleastropy.SkyCoord instead of ra and dec or a combine multiple flags into a config- dataclass.
Comments like this that don't add any information beyond what the function name already contains should be more about the bigger picture and why something is done or just omitted.
type-hints would be a nice addition, even though the types are already mentioned in the docstring. It allows for automatic checks and shows users of your library squiggly lines in their IDE for some of the mistakes they might make.
I'm a bit confused by the working dir mechanism. Why not just let the user use os.chdir()? In case you want to be more flexible, you could have a global configuration object that has something like working_directory: pathlib.Path and then use it whenever you want to write something (like hdu.writeto(config.working_directory/'foo.fits')). In my understanding "dunder" varriables (__foo__) are reserved for file metadata or to inspect internal state but should not be required for the program to run.
Stuff like filter names that are restricted to certain values should ideally be constants or enums. Or at least their possible values should be listed in the docstring with the parameter explanation.
I'll also open this as a place for comments that aren't really relevant to the submission criteria but which I nevertheless hope to be constructive criticism more in line with a general code review.
As a general remark, be aware that most code will be read more often than it is written and reducing "mental load" you place on your audience (which includes future you) is a big factor to the time and nerves it will cost them to understand what is going on. I'd wager a guess and say you and others are also a lot more likely to contribute if they feel "at home". There's a few things that help with that
size->image_size
,pos->host_galaxy_coordinates
). But keep in mind that at 4-5 layers of indentation, things become really hard to follow, so still set a reasonable limit like 120 chars.astropy.SkyCoord
instead ofra
anddec
or a combine multiple flags into a config-dataclass
.Comments like this that don't add any information beyond what the function name already contains should be more about the bigger picture and why something is done or just omitted.
type-hints would be a nice addition, even though the types are already mentioned in the docstring. It allows for automatic checks and shows users of your library squiggly lines in their IDE for some of the mistakes they might make.
I'm a bit confused by the working dir mechanism. Why not just let the user use
os.chdir()
? In case you want to be more flexible, you could have a global configuration object that has something likeworking_directory: pathlib.Path
and then use it whenever you want to write something (likehdu.writeto(config.working_directory/'foo.fits')
). In my understanding "dunder" varriables (__foo__
) are reserved for file metadata or to inspect internal state but should not be required for the program to run.Stuff like filter names that are restricted to certain values should ideally be constants or enums. Or at least their possible values should be listed in the docstring with the parameter explanation.