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

Apply code style rules to soss_extract #9156

Merged
merged 10 commits into from
Feb 10, 2025

Conversation

emolter
Copy link
Collaborator

@emolter emolter commented Feb 7, 2025

Partially resolves JP-3860

This PR updates the soss_extract submodule to comply with the new code style rules.

Since soss_extract is nestled inside extract_1d, I had to add some ugly per-file-ignores to .pre-commit-config.yaml and .ruff.toml; these can be removed once the rest of extract_1d is updated to follow the style rules.

As a procedural note, I added a line to the epic labeled soss_extract and assigned it to myself, then removed my name from the assignment list next to extract_1d so someone else could pick up the rest of the step.

Tasks

  • request a review from someone specific, to avoid making the maintainers review every PR
  • add a build milestone, i.e. Build 11.3 (use the latest build if not sure)
  • Does this PR change user-facing code / API? (if not, label with no-changelog-entry-needed)
    • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
    • update or add relevant tests
    • update relevant docstrings and / or docs/ page
    • start a regression test and include a link to the running job (click here for instructions)
      • Do truth files need to be updated ("okified")?
        • after the reviewer has approved these changes, run okify_regtests to update the truth files
  • if a JIRA ticket exists, make sure it is resolved properly
news fragment change types...
  • changes/<PR#>.general.rst: infrastructure or miscellaneous change
  • changes/<PR#>.docs.rst
  • changes/<PR#>.stpipe.rst
  • changes/<PR#>.datamodels.rst
  • changes/<PR#>.scripts.rst
  • changes/<PR#>.set_telescope_pointing.rst
  • changes/<PR#>.pipeline.rst

stage 1

  • changes/<PR#>.group_scale.rst
  • changes/<PR#>.dq_init.rst
  • changes/<PR#>.emicorr.rst
  • changes/<PR#>.saturation.rst
  • changes/<PR#>.ipc.rst
  • changes/<PR#>.firstframe.rst
  • changes/<PR#>.lastframe.rst
  • changes/<PR#>.reset.rst
  • changes/<PR#>.superbias.rst
  • changes/<PR#>.refpix.rst
  • changes/<PR#>.linearity.rst
  • changes/<PR#>.rscd.rst
  • changes/<PR#>.persistence.rst
  • changes/<PR#>.dark_current.rst
  • changes/<PR#>.charge_migration.rst
  • changes/<PR#>.jump.rst
  • changes/<PR#>.clean_flicker_noise.rst
  • changes/<PR#>.ramp_fitting.rst
  • changes/<PR#>.gain_scale.rst

stage 2

  • changes/<PR#>.assign_wcs.rst
  • changes/<PR#>.badpix_selfcal.rst
  • changes/<PR#>.msaflagopen.rst
  • changes/<PR#>.nsclean.rst
  • changes/<PR#>.imprint.rst
  • changes/<PR#>.background.rst
  • changes/<PR#>.extract_2d.rst
  • changes/<PR#>.master_background.rst
  • changes/<PR#>.wavecorr.rst
  • changes/<PR#>.srctype.rst
  • changes/<PR#>.straylight.rst
  • changes/<PR#>.wfss_contam.rst
  • changes/<PR#>.flatfield.rst
  • changes/<PR#>.fringe.rst
  • changes/<PR#>.pathloss.rst
  • changes/<PR#>.barshadow.rst
  • changes/<PR#>.photom.rst
  • changes/<PR#>.pixel_replace.rst
  • changes/<PR#>.resample_spec.rst
  • changes/<PR#>.residual_fringe.rst
  • changes/<PR#>.cube_build.rst
  • changes/<PR#>.extract_1d.rst
  • changes/<PR#>.resample.rst

stage 3

  • changes/<PR#>.assign_mtwcs.rst
  • changes/<PR#>.mrs_imatch.rst
  • changes/<PR#>.tweakreg.rst
  • changes/<PR#>.skymatch.rst
  • changes/<PR#>.exp_to_source.rst
  • changes/<PR#>.outlier_detection.rst
  • changes/<PR#>.tso_photometry.rst
  • changes/<PR#>.stack_refs.rst
  • changes/<PR#>.align_refs.rst
  • changes/<PR#>.klip.rst
  • changes/<PR#>.spectral_leak.rst
  • changes/<PR#>.source_catalog.rst
  • changes/<PR#>.combine_1d.rst
  • changes/<PR#>.ami.rst

other

  • changes/<PR#>.wfs_combine.rst
  • changes/<PR#>.white_light.rst
  • changes/<PR#>.cube_skymatch.rst
  • changes/<PR#>.engdb_tools.rst
  • changes/<PR#>.guider_cds.rst

@emolter
Copy link
Collaborator Author

emolter commented Feb 7, 2025

regression tests https://github.com/spacetelescope/RegressionTests/actions/runs/13204660540

Errors are unrelated and match the FGS guider errors on main.

@emolter emolter requested a review from tapastro February 7, 2025 17:14
Copy link

codecov bot commented Feb 7, 2025

Codecov Report

Attention: Patch coverage is 61.70732% with 157 lines in your changes missing coverage. Please review.

Project coverage is 73.76%. Comparing base (a4175b4) to head (c8517b9).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
jwst/extract_1d/soss_extract/soss_extract.py 27.43% 119 Missing ⚠️
jwst/extract_1d/soss_extract/atoca_utils.py 83.50% 16 Missing ⚠️
jwst/extract_1d/soss_extract/pastasoss.py 30.00% 14 Missing ⚠️
jwst/extract_1d/soss_extract/atoca.py 91.22% 5 Missing ⚠️
jwst/extract_1d/soss_extract/soss_boxextract.py 77.77% 2 Missing ⚠️
jwst/extract_1d/soss_extract/soss_syscor.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #9156   +/-   ##
=======================================
  Coverage   73.76%   73.76%           
=======================================
  Files         372      372           
  Lines       37276    37276           
=======================================
  Hits        27498    27498           
  Misses       9778     9778           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@emolter emolter marked this pull request as ready for review February 7, 2025 18:35
@emolter emolter requested review from a team as code owners February 7, 2025 18:35
Copy link
Collaborator

@melanieclarke melanieclarke left a comment

Choose a reason for hiding this comment

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

Lots here, thanks for getting it cleaned up. I can take the rest of the extract1d module, when this one is in.

I spotted a few typos and other small things while reading through.

jwst/extract_1d/soss_extract/atoca.py Outdated Show resolved Hide resolved
jwst/extract_1d/soss_extract/atoca.py Outdated Show resolved Hide resolved
jwst/extract_1d/soss_extract/atoca.py Outdated Show resolved Hide resolved
jwst/extract_1d/soss_extract/atoca_utils.py Show resolved Hide resolved
jwst/extract_1d/soss_extract/atoca_utils.py Outdated Show resolved Hide resolved
jwst/extract_1d/soss_extract/atoca_utils.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@melanieclarke melanieclarke left a comment

Choose a reason for hiding this comment

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

I think these changes are uncontroversial enough we can go ahead and get them merged. We'll follow up on more documentation updates later.

@emolter emolter merged commit b68974f into spacetelescope:main Feb 10, 2025
28 of 29 checks passed
@emolter emolter deleted the JP-3860-soss-extract branch February 10, 2025 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants