sbpy icon indicating copy to clipboard operation
sbpy copied to clipboard

Standardize the dimension specification in Field Name List and improve tests

Open jianyangli opened this issue 6 years ago • 1 comments

This is a request for

  • [ ] a new feature
  • [x] an enhancement to existing sbpy functionality
  • [ ] somethings else: [explain here]

The requested changes will be implemented by

  • [ ] me
  • [x] the sbpy developers

High-level concept [Provide a short description of the scientific or technical problem that your new feature would address.]

As suggested by @mkelley in PR #218:

  1. The dimension in the column in Field Name List should be standardized to be directly recognizable by astropy.units.
  2. Add tests to check dimensions of all fields in sbpy.data.

Explain the relevance to sbpy [Why is this relevant to sbpy but not to any other astropy affiliated or other package?]

Simplify dimension check in all sbpy code that uses DataClass.

Proposal details [Provide details technical and scientific details on your request. Is there a publication that uses the same method as requested here? If you plan to implement this feature yourself: How will you implement the code? ]

The standardized dimension string should be either valid 'physical type' as defined in astropy.units, or a base unit that can be converted to a valid astropy unit by a astropy.unit.Unit() call. Exceptions can exist, such as when the field is a astropy.time.Time instance. Standard logarithmic unit and astropy.units.MagUnit are also exceptions due their flexible physical bases.

Example (pseudo-)code [Only required if you plan to implement this feature yourself: Provide a short example demonstrating the expected API]

jianyangli avatar Aug 10 '19 02:08 jianyangli

I think I've mostly addressed this with #275. However, I was not aware of astropy's PhysicalTime and locally implemented a similar solution. So, I would prefer using astropy machinery over our own, so I would like to do that. Thanks for pointing this out.

mkelley avatar Sep 27 '21 00:09 mkelley