Skip to content

New backplanes#161

Merged
jnspitale merged 145 commits intomainfrom
new_backplanes
Mar 20, 2025
Merged

New backplanes#161
jnspitale merged 145 commits intomainfrom
new_backplanes

Conversation

@jnspitale
Copy link
Copy Markdown
Collaborator

Mostly updates for the new backplanes. There is also a fix to the Galileo SSI module to properly recognize a null CUT_OUT_WINDOW.

I don't know why there is a mod to nircam. So maybe that shouldn't be there.

 - compute mask using window given in label.
 - add mask to observation.
- added gold master tests for cassini uvis -- fails because uvis needs updating
- added gold master tests for cassini vims -- fails because vims needs updating
- added gold master tests for juno jiram  -- not yet tested
- made junocam gold master unit test consistent w style of other hosts
- added gold master tests for voyager ISS  -- not yet tested
- removed backplane/unittester_support.py, backplane_exercises.py
…does not

  currently allow arguments to be passed in this way.
- import standard_obs in junocam/__init__.py and junocam/__init__goldmaster.py
- gm.override modified to be observation-specific
- added gm.execute_standard_command()
- gm.execute_standard_unittest() runs all standard observations if name not given
- added exclude arguments to gm.execute_standard_unittest() and
  gm.execute_standard_command()
Copy link
Copy Markdown
Collaborator

@markshowalter markshowalter left a comment

Choose a reason for hiding this comment

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

This is good code but I have a bunch of comments, as usual.

Comment thread oops/backplane/limb.py
Comment thread oops/backplane/limb.py
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/hosts/galileo/ssi/__init__.py Outdated
Comment thread oops/hosts/jwst/nircam/__init__.py
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py
@jnspitale
Copy link
Copy Markdown
Collaborator Author

Just pushed new commit with the requested changes.

Copy link
Copy Markdown
Collaborator

@markshowalter markshowalter left a comment

Choose a reason for hiding this comment

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

Scattered remaining comments.

Comment thread oops/backplane/pixel.py
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/ring.py Outdated
Comment thread oops/hosts/galileo/ssi/__init__.py Outdated
Comment thread oops/backplane/ring.py Outdated
- change asserts to raises in ring.py and pixel.py
- other changes in response to Mark's comments
@markshowalter
Copy link
Copy Markdown
Collaborator

I know this isn't ready for review, but I have two quick comments.

  • It's inconsistent to use axis = "u" or "v" for body_diameter_in_pixels() but axis = "x" or "y" in center_coordinates(). I think the latter is preferred, because this is a user-facing function and users are more likely to know what "x" and "y" mean. The u/v notation is mostly just internal to the source code.
  • assert statements are for internal self-checks in the code, not for user-facing tests. ValueError is defined in the Python standard as the exception to raise when a user passes an invalid input to a function. So please change your assert statements to tests that raise ValueError with an explicit message.

@jnspitale
Copy link
Copy Markdown
Collaborator Author

The asserts should already be gone.

@jnspitale
Copy link
Copy Markdown
Collaborator Author

I would advocate changing to uv in center_coordinates. OPUS users only see the column names, which are written as X and Y.

@jnspitale jnspitale marked this pull request as ready for review March 20, 2025 18:42
Copy link
Copy Markdown
Collaborator

@markshowalter markshowalter left a comment

Choose a reason for hiding this comment

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

See comments.

Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/pixel.py Outdated
Comment thread oops/backplane/ring.py Outdated
Comment thread oops/backplane/pixel.py Outdated
@jnspitale jnspitale merged commit c9f1a25 into main Mar 20, 2025
13 checks passed
@jnspitale jnspitale deleted the new_backplanes branch March 20, 2025 22:58
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.

4 participants