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

ENH: Use astropy units and Quantity #16

Open
pllim opened this issue Aug 11, 2020 · 0 comments
Open

ENH: Use astropy units and Quantity #16

pllim opened this issue Aug 11, 2020 · 0 comments
Labels
enhancement New feature or request

Comments

@pllim
Copy link
Collaborator

pllim commented Aug 11, 2020

astropy units and Quantity is useful because it keeps track of the units for you and works pretty well with Numpy arrays (except for some known issues). Things like trapezoid_fit (see also #14) could benefit from it.

Pros:

  • Automatic units bookkeeping. For example, we can get rid of warnings like "must pass in value as hours, not days" and reduce to risk of wrong calculations due to user error. You would be able to pass in either hours or days (or anything that can convert to them for that matter) and not have to worry about wrong results.
  • Works well with Numpy arrays.

Cons:

  • Extra computation overhead. There are some performance tips but some overhead is unavoidable. Units bookkeeping does not come for free.
  • Risk of introducing new bugs during refactoring. Some existing magic numbers need to be well understood and/or eliminated. For example, is that 1.0e6 used to convert units or for something else?
  • scipy.optimize cannot understand astropy units and Quantity, so the units still need to be stripped before that is called and then re-applied. It is important that we convert the Quantity to expected units before stripping the units.

Do the benefits outweigh the costs?

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

No branches or pull requests

1 participant