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-35173: Add descriptions to CcdVisit and Visit Tables #65

Merged
merged 1 commit into from Jun 23, 2022

Conversation

cmsaunders
Copy link
Contributor

No description provided.

@cmsaunders cmsaunders force-pushed the tickets/DM-35173 branch 2 times, most recently from d04e603 to 9a29728 Compare June 21, 2022 15:04
- name: band
'@id': '#Visit.band'
datatype: char
length: 1
description: ''
votable:arraysize: "*"
Copy link
Member

Choose a reason for hiding this comment

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

Portal needs to keep this votable:arraysize line in order to work around a bug.

- name: band
'@id': '#CcdVisit.band'
datatype: char
length: 1
description: ''
votable:arraysize: "*"
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above, we need this line at the moment.

- name: expMidpt
'@id': '#Visit.expMidpt'
datatype: timestamp
length: 6
mysql:datatype: DATETIME(6)
description: ''
description: Midpoint for exposure at the fiducial center of the focal plane array.
Copy link
Member

Choose a reason for hiding this comment

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

Just to make it super obvious, it's probably worth saying "Midpoint time" or otherwise making it clear that it's the time.

- name: azimuth
'@id': '#Visit.azimuth'
datatype: double
description: ''
description: Azimuth of focal plane center.
Copy link
Member

Choose a reason for hiding this comment

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

Is this at the exposure midpoint?

- name: expTime
'@id': '#Visit.expTime'
datatype: double
description: ''
description: Average duration of exposure, accurate to 10ms.
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how to clarify this succinctly, but users might not know that this means spatially-averaged. Might be ok to leave off the average part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just changed this to "Spatially-averaged duration of exposure..."

- name: decl
'@id': '#Visit.decl'
datatype: double
description: ''
description: Decl of focal plane center
Copy link
Member

Choose a reason for hiding this comment

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

I think it's worth writing out declination.

description: ''
description: Start of the exposure at the fiducial center of the focal plane array,
TAI, accurate to 10ms.
defaultValue: CURRENT_TIMESTAMP
Copy link
Member

Choose a reason for hiding this comment

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

What does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something that was in the BaselineSchema, but I think I will take it out, because I'm not sure how it could be helpful to have a default value for obsStart. It seems more likely to cause confusion.

@cmsaunders cmsaunders merged commit 50f8fa3 into main Jun 23, 2022
@cmsaunders cmsaunders deleted the tickets/DM-35173 branch June 23, 2022 15:04
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