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

Error with missing units #2

Open
saewitz opened this issue Oct 9, 2021 · 3 comments
Open

Error with missing units #2

saewitz opened this issue Oct 9, 2021 · 3 comments

Comments

@saewitz
Copy link

saewitz commented Oct 9, 2021

Hi, thanks for this library! I ran into a few errors with missing units on a label data file from hirise, and added these lines:

from astropy import units
unit_meters_pixel = units.def_unit('meters/pixel', units.meter / units.pixel)
unit_local_day    = units.def_unit('localday/24', units.day / 24)
unit_bytes    = units.def_unit('bytes', units.byte)
units.add_enabled_units([unit_meters_pixel, unit_local_day, unit_bytes])

Reference to astropy defining units: https://docs.astropy.org/en/latest/units/combining_and_defining.html

I'm pretty sure unit_local_day is wrong, because it represents a 24-hour day according to the astropy documentation, but local day might be defined differently on labels. This could cause issues.

I also might be totally wrong that this needs to be added -- I'm new to this kind of data. If this is something that needs to be done, it might be good to add it to this library with fixes.

@mkelley
Copy link
Owner

mkelley commented Oct 11, 2021

Thanks for your interest. I'm glad you find it useful.

Yes, having a unit that varies based on context would be challenging to add. I'm thinking that I can define "localday" as a new IrreducibleUnit, and then one can convert to time using equivalencies:

>>> localday = u.def_unit('localday')                                                                                                          
>>> martian_time = u.Equivalency([(localday, u.s, lambda x: x * 88775.244, lambda x: x / 88775.244)])                                          
>>> (1 * localday).to('s', martian_time)                                                                                                       
Out[21]: <Quantity 88775.244 s>

Regarding the other units: I see that plural names are generally allowed by PDS3, so I'll see if I can sanitize the PDS3 the string before converting to astropy units.

@mkelley
Copy link
Owner

mkelley commented Oct 11, 2021

OK, I've made an update to address the units, but the bit mask isn't parsing. I'll need to work on that tomorrow.

@saewitz
Copy link
Author

saewitz commented Oct 11, 2021

Cool, thank you! I ran into that issue as well, and just commented it out for now since I didn't need it.

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

2 participants