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

Add setting of number_extended_events #178

Merged

Conversation

mwregan2
Copy link
Collaborator

@mwregan2 mwregan2 commented Jul 5, 2023

Related to JP-3096

This PR fixes the population of the new EXTNCRS keyword, which was not being set when in single processing mode.

Checklist

  • added entry in CHANGES.rst (either in Bug Fixes or Changes to API)
  • updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)

This was not being set when in single processing mode.
@mwregan2 mwregan2 requested a review from a team as a code owner July 5, 2023 18:24
@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Patch coverage: 22.22% and project coverage change: -0.06 ⚠️

Comparison is base (e0c6f69) 74.46% compared to head (eb5255d) 74.41%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
- Coverage   74.46%   74.41%   -0.06%     
==========================================
  Files          29       29              
  Lines        5937     5944       +7     
==========================================
+ Hits         4421     4423       +2     
- Misses       1516     1521       +5     
Impacted Files Coverage Δ
src/stcal/jump/jump.py 62.92% <0.00%> (-0.87%) ⬇️
tests/test_jump.py 88.46% <50.00%> (-0.32%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@kmacdonald-stsci kmacdonald-stsci left a comment

Choose a reason for hiding this comment

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

The changes look fine, but there are code failures and CI failures that need to be corrected.

CHANGES.rst Outdated Show resolved Hide resolved
@hbushouse
Copy link
Collaborator

Don't understand why the one pytest that was unskipped is causing failures in the CI tests (claims it can't find the input data file, which is clearly present in the data directory), but runs fine when I test it locally. So just to get the CI tests to work I marked it to be skipped again.

@hbushouse
Copy link
Collaborator

@ddavis-stsci Can you please review for Roman?

@hbushouse hbushouse added the bug Something isn't working label Jul 7, 2023
Copy link
Collaborator

@hbushouse hbushouse left a comment

Choose a reason for hiding this comment

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

All CI tests are passing now. So I approve.

Copy link
Collaborator

@ddavis-stsci ddavis-stsci left a comment

Choose a reason for hiding this comment

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

LGTM, this should be skipped for romancal and romancal regression tests pass with this update.
https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-Developers-Pull-Requests/273/

@hbushouse
Copy link
Collaborator

OK, it looks like we're all in agreement. Merging now.

@hbushouse hbushouse merged commit bebfad8 into spacetelescope:main Jul 7, 2023
17 of 19 checks passed
mairanteodoro pushed a commit to mairanteodoro/stcal that referenced this pull request Jul 10, 2023
* Add setting of number_extended_events

This was not being set when in single processing mode.

* Update CHANGES.rst

* Update CHANGES.rst

* Update test_jump.py

---------

Co-authored-by: Howard Bushouse <bushouse@stsci.edu>
mairanteodoro pushed a commit that referenced this pull request Jul 17, 2023
* Add setting of number_extended_events

This was not being set when in single processing mode.

* Update CHANGES.rst

* Update CHANGES.rst

* Update test_jump.py

---------

Co-authored-by: Howard Bushouse <bushouse@stsci.edu>
nden added a commit that referenced this pull request Aug 2, 2023
* Add methods necessary for resample.

* Small changes to accommodate both JWST and RST datamodels.

* Add CHANGES.rst entry.

* Add PR number to CHANGES.rst entry.

* use protocols

* temp

* add dependencies

* add CI testing to alignment branch

* add CI testing to alignment branch

* fix test

* Add setting of number_extended_events (#178)

* Add setting of number_extended_events

This was not being set when in single processing mode.

* Update CHANGES.rst

* Update CHANGES.rst

* Update test_jump.py

---------

Co-authored-by: Howard Bushouse <bushouse@stsci.edu>

* Rename variables with FITS keywords names.

* Style reformating.

* Update docs to include stcal.alignment.

* Revert "Rename variables with FITS keywords names."

This reverts commit 0eea0e5.

* Updates to address most comments.

* Fix misplaced comment.

* Code refactoring and additional unit test.

* Silencing style check warning F403.

* Small style refactoring.

* Set minimum asdf version.

* Update matplotlib's inventory URL.

* Set lower version for gwcs.

* Bump astropy version to >= 5.1.

* Fix docs style issues.

* Style check fix.

* Fix issue with bounding boxes in tests.

* Fix issue with bounding boxes in tests.

---------

Co-authored-by: Nadia Dencheva <nadia.dencheva@gmail.com>
Co-authored-by: mwregan2 <mregan@stsci.edu>
Co-authored-by: Howard Bushouse <bushouse@stsci.edu>
mairanteodoro added a commit that referenced this pull request Aug 22, 2023
* Add methods necessary for resample.

* Small changes to accommodate both JWST and RST datamodels.

* Add CHANGES.rst entry.

* Add PR number to CHANGES.rst entry.

* use protocols

* temp

* add dependencies

* add CI testing to alignment branch

* add CI testing to alignment branch

* fix test

* Add setting of number_extended_events (#178)

* Add setting of number_extended_events

This was not being set when in single processing mode.

* Update CHANGES.rst

* Update CHANGES.rst

* Update test_jump.py

---------

Co-authored-by: Howard Bushouse <bushouse@stsci.edu>

* Rename variables with FITS keywords names.

* Style reformating.

* Update docs to include stcal.alignment.

* Revert "Rename variables with FITS keywords names."

This reverts commit 0eea0e5.

* Updates to address most comments.

* Fix misplaced comment.

* Code refactoring and additional unit test.

* Silencing style check warning F403.

* Small style refactoring.

* Set minimum asdf version.

* Update matplotlib's inventory URL.

* Set lower version for gwcs.

* Bump astropy version to >= 5.1.

* Fix docs style issues.

* Style check fix.

* Fix issue with bounding boxes in tests.

* Fix issue with bounding boxes in tests.

* Add new methods required by resample_step.

* Small refactoring and clean ups.

* RCAL-632: add unit tests for new methods.

* Fix style check errors.

* Set sphinx version for compatibility with sphinx-rtd-theme.

* Fix style issues.

* Revert reproject commit.

---------

Co-authored-by: Nadia Dencheva <nadia.dencheva@gmail.com>
Co-authored-by: mwregan2 <mregan@stsci.edu>
Co-authored-by: Howard Bushouse <bushouse@stsci.edu>
mairanteodoro added a commit that referenced this pull request Aug 23, 2023
* Add methods necessary for resample.

* Small changes to accommodate both JWST and RST datamodels.

* Add CHANGES.rst entry.

* Add PR number to CHANGES.rst entry.

* use protocols

* temp

* add dependencies

* add CI testing to alignment branch

* add CI testing to alignment branch

* fix test

* Add setting of number_extended_events (#178)

* Add setting of number_extended_events

This was not being set when in single processing mode.

* Update CHANGES.rst

* Update CHANGES.rst

* Update test_jump.py

---------

Co-authored-by: Howard Bushouse <bushouse@stsci.edu>

* Rename variables with FITS keywords names.

* Style reformating.

* Update docs to include stcal.alignment.

* Revert "Rename variables with FITS keywords names."

This reverts commit 0eea0e5.

* Updates to address most comments.

* Fix misplaced comment.

* Code refactoring and additional unit test.

* Silencing style check warning F403.

* Small style refactoring.

* Set minimum asdf version.

* Update matplotlib's inventory URL.

* Set lower version for gwcs.

* Bump astropy version to >= 5.1.

* Fix docs style issues.

* Style check fix.

* Fix issue with bounding boxes in tests.

* Fix issue with bounding boxes in tests.

* Add new methods required by resample_step.

* Small refactoring and clean ups.

* RCAL-632: add unit tests for new methods.

* Fix style check errors.

* Set sphinx version for compatibility with sphinx-rtd-theme.

* Fix style issues.

* Revert reproject commit.

* Small changes to accommodate both JWST and RST datamodels.

* fix test

* Rename variables with FITS keywords names.

* Revert "Rename variables with FITS keywords names."

This reverts commit 0eea0e5.

* Code refactoring and additional unit test.

* Bump astropy version to >= 5.1.

* Fix docs style issues.

* Small refactoring and clean ups.

* Fix style issues.

* Revert reproject commit.

* Revert "Revert reproject commit."

This reverts commit 9ac8694.

* Fix issue caused by bad conflict resolution.

* Fix duplicate code.

---------

Co-authored-by: Nadia Dencheva <nadia.dencheva@gmail.com>
Co-authored-by: mwregan2 <mregan@stsci.edu>
Co-authored-by: Howard Bushouse <bushouse@stsci.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working jump testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants