-
Notifications
You must be signed in to change notification settings - Fork 27
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
CalH5 Memo #1462
CalH5 Memo #1462
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1462 +/- ##
=======================================
Coverage 99.92% 99.92%
=======================================
Files 61 61
Lines 21319 21319
=======================================
Hits 21304 21304
Misses 15 15 ☔ View full report in Codecov by Sentry. |
I haven't yet added the pdfs to this PR since I figured there'd be updates needed, but I'm attaching them here to help with review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is looking good @bhazelton! One comment of moderate significance that I think should be fast to address, otherwise I think this is looking good to go.
various groups. We note in parenthesis the corresponding attribute of a UVCal | ||
object. Note that in nearly all cases, the names are coincident, to make things | ||
as transparent as possible to the user. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may suggest providing here a brief synopsis of the different types of calibration solutions (delay vs gains; wideband versus freq-dependent/bandpass), just to help give some context up front (and also define terms like "wideband")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a paragraph in the introduction about this, let me know if it's sufficiently detailed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
found a couple typos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me, thanks @bhazelton!
ok, I'll add the pdfs and then get you to re-review @kartographer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dub thee memos plenipotentiary and extraordinary!
Description
Add a memo describing the CalH5 file format, along the lines of the UVH5 memo.
Also added several missing optional header items to the UVH5 memo and did some other doc updates.
Motivation and Context
closes #1424
Types of changes
Checklist:
Documentation change checklist: