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

DM-41923: Fix unit errors in DP0.1 schema #167

Merged
merged 1 commit into from Dec 4, 2023
Merged

Conversation

JeremyMcCormick
Copy link
Collaborator

The unit "pixels" is changed to "pixel". The unit "asec^2" is changed to "arcsec^2". The unit "asec" is changed to "arcsec". The new values were checked for validity using the astropy Unit class.

Copy link
Collaborator

@gpdf gpdf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -8359,23 +8359,23 @@ tables:
mysql:datatype: DOUBLE
tap:column_index: 999
tap:principal: 0
fits:tunit: nmgy*asec^2
fits:tunit: nmgy*arcsec^2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why "nmgy" is accepted by the Astropy validator. It's not in VOUnit or FITS, whether interpreted as a single unit or as nano-"mgy".

The "nanomaggie" is a well-recognized unit in the field from a larger perspective (see, e.g., https://www.sdss3.org/dr8/algorithms/magnitudes.php), so perhaps it was added to Astropy to support this even though it's not in the actual community standards.

Anyway, I am happy to leave well enough along here, especially since it's just the near-deprecated DP0.1.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid any ambiguity, I meant "'nanomaggie' is the English name of a well-known informal unit", not "the string 'nanomaggie' is standardized".

There appears to be no community standard for a machine-readable name for this concept. I am happy to leave it as "nmgy" especially as it appears only in DP0.1.

The unit "pixels" is changed to "pixel". The unit "asec^2" is changed
to "arcsec^2". The unit "asec" is changed to "arcsec". The new values
were checked for validity using the astropy Unit class.
@JeremyMcCormick JeremyMcCormick merged commit 6725770 into main Dec 4, 2023
4 checks passed
@JeremyMcCormick JeremyMcCormick deleted the tickets/DM-41923 branch December 4, 2023 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants