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

SWIFT_BAT_GRB_POS_ACK conversion #36

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

athish-thiru
Copy link
Contributor

All SWIFT_BAT_* notices have a very similar similar conversion with just a few differing/missing fields. So if the format is acceptable I could commit the rest of the conversions to this pull request as well.

The following keywords do not exist in the core schema for SWIFT_BAT_GRB_POS_ACK:

  • n_events
  • image_peak
  • bankground_events
  • background_start_time
  • background_duration
  • trigger_index
  • catalog_number

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

Please remove the xfail mark for the unit test.

@lpsinger
Copy link
Member

lpsinger commented Aug 7, 2024

@parsotat, would you please review this?

…sary equalities

changed Unused to intentionally omitted

reformated comments
Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

Did @parsotat ever take a look at this?

Comment on lines 43 to 45
comments = "\n".join(
[val for (key, val) in flag_descriptions.items() if (soln_status_bits[key])]
)
Copy link
Member

Choose a reason for hiding this comment

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

Please capture these flags in a machine-readable way, not as comments.

Copy link
Contributor Author

@athish-thiru athish-thiru Aug 26, 2024

Choose a reason for hiding this comment

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

I use additional_info to capture the data stored in the COMMENTS: fields in the text notices. Some of these are redundant with the data in the notices and can be removed(I've mainly added them for completeness). However, some of the these comments I think may be too complex to easily translate into a field like "StarTracker not locked so trigger probably bogus" or "This was orginally a SubTresh, but it is now converted to a real BAT_POS.".

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

Successfully merging this pull request may close these issues.

2 participants