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

Allow group specifications as integers or tuples #1612

Merged
merged 1 commit into from
Aug 18, 2023

Conversation

mcara
Copy link
Member

@mcara mcara commented Jul 13, 2023

This PR fixes a bug reported in INC0189304 that, contrary to the documentation, setting group argument would result in a crash. In addition, now it is allowed to set group to a Python list of integers (extension numbers) and list of tuples of extension name and extension version ((extname, extver)).

@mcara mcara requested review from mdlpstsci and a team as code owners July 13, 2023 09:07
@mcara mcara self-assigned this Jul 13, 2023
@mcara mcara added the bug label Jul 13, 2023
@mcara mcara added this to the Better Awesomeness milestone Jul 13, 2023
@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Patch coverage: 14.81% and project coverage change: -1.69% ⚠️

Comparison is base (d37ee76) 34.01% compared to head (124fe98) 32.33%.

❗ Current head 124fe98 differs from pull request most recent head ce5cac7. Consider uploading reports for the commit ce5cac7 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1612      +/-   ##
==========================================
- Coverage   34.01%   32.33%   -1.69%     
==========================================
  Files         126      159      +33     
  Lines       31124    35088    +3964     
  Branches     5794        0    -5794     
==========================================
+ Hits        10588    11347     +759     
- Misses      19333    23741    +4408     
+ Partials     1203        0    -1203     
Files Changed Coverage Δ
drizzlepac/util.py 56.10% <6.25%> (+1.21%) ⬆️
drizzlepac/processInput.py 50.56% <80.00%> (+2.70%) ⬆️
drizzlepac/imageObject.py 68.21% <100.00%> (+5.80%) ⬆️

... and 129 files with indirect coverage changes

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

@mcara mcara requested a review from s-goldman July 13, 2023 17:36
s-goldman
s-goldman previously approved these changes Jul 18, 2023
Copy link
Collaborator

@s-goldman s-goldman left a comment

Choose a reason for hiding this comment

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

Code looks good. If you could, however, please add explicitly in the docstring descriptions of the three inputs are and their allowed types, and what is returned.

@mcara mcara merged commit b68c9ab into spacetelescope:master Aug 18, 2023
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants