Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use astropy.units #14

Open
wtbarnes opened this issue Sep 3, 2021 · 3 comments
Open

Use astropy.units #14

wtbarnes opened this issue Sep 3, 2021 · 3 comments

Comments

@wtbarnes
Copy link
Contributor

wtbarnes commented Sep 3, 2021

There are several places throughout the package where using units would be extremely helpful to both the user and in reducing the number of validation checks needed, e.g. ccd_offset, rot_xy. I think it would be a good idea to impose astropy units wherever possible. This would also help to increase interoperability with sunpy and the sunpy affiliated package ecosystem.

For a formal description of what this looks like in sunpy, see SEP-0003

@PaulJWright
Copy link

This would certainly help clear up confusion in ccd_offset, see #57. I am not going to ask this for the JOSS review, but I agree with @wtbarnes here.

@MJWeberg
Copy link
Collaborator

Thanks for the reminder! Implementing units for these functions is on our todo list, but it is fairly low-priority since these functions are used internally and should rarely, if ever, be called by users directly (additionally, ccd_offset is one of the only bits of code we ported directly over from IDL, so we want to make sure to maintain consistency if possible).

@wtbarnes
Copy link
Contributor Author

Fair point about the internal use of those functions. I think I had just listed those as examples. Are there other user-facing functions that would benefit from taking units as inputs and giving units as outputs?

One place where this is already done is the EISMap class properties, e.g. the wavelength property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants