Skip to content

Handle np.average with different shape weights and input array and returning sum of weights #19055

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.

Sign up for GitHub

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

Jump to bottom
Merged
mhvk merged 7 commits into from
Dec 27, 2025
Merged

Handle np.average with different shape weights and input array and returning sum of weights #19055

mhvk merged 7 commits into from
Dec 27, 2025
+52 −1

Conversation

@kyleaoman
Copy link
Contributor

@kyleaoman kyleaoman commented Dec 15, 2025

Description

This pull request is to address an issue where the np.average function strips the units from the optionally-returned sum of weights. A complete description and instructions to reproduce are at #19054

The fix is simple: I've just moved np.average out of SUBCLASS_SAFE_FUNCTIONS and into FUNCTION_HELPERS and written a suitable helper. I've also added a regression test.

Fixes #19054

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@github-actions github-actions bot added the units label Dec 15, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 15, 2025

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@pllim pllim added the Bug label Dec 15, 2025
@pllim pllim added this to the v7.2.1 milestone Dec 15, 2025
@pllim pllim added the backport-v7.2.x on-merge: backport to v7.2.x label Dec 15, 2025
@pllim pllim requested a review from mhvk December 15, 2025 16:53
@pllim
Copy link
Member

pllim commented Dec 15, 2025

Thanks! Please add a bugfix change log.

https://github.com/astropy/astropy/blob/main/docs/changes/README.rst

mhvk
mhvk reviewed Dec 26, 2025
View reviewed changes
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Thanks very much for both reporting the original issue and making a PR with the fix!
See my few in-line comments. Also, as @pllim noted, would be good to have a changelog fragment.

@kyleaoman
Copy link
Contributor Author

kyleaoman commented Dec 26, 2025

Hi @mhvk, thanks for the review. I've implemented those suggestions, and the changelog fragment was already in place (0643c95).

@kyleaoman
Copy link
Contributor Author

kyleaoman commented Dec 26, 2025

Um, merging main was a mistake - rebased and force-pushed instead. Sorry for any confusion!

mhvk
mhvk reviewed Dec 27, 2025
View reviewed changes
astropy/units/quantity_helper/function_helpers.py Outdated

a = _as_quantity(a)
a_value, a_unit = a.value, a.unit
weights = _as_quantity(weights)
Copy link
Contributor

@mhvk mhvk Dec 27, 2025

Choose a reason for hiding this comment

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

This raises an error for weights=None, no?

Hmm, just checked, that indeed errors. There should have been a test for that... Let me push the fix directly to your branch.

mhvk
mhvk approved these changes Dec 27, 2025
View reviewed changes
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Thanks, @kyleaoman! Approving with my small modification, and will set to merge.

@mhvk mhvk enabled auto-merge (squash) December 27, 2025 18:48
@mhvk mhvk merged commit 8e04a63 into astropy:main Dec 27, 2025
33 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Dec 27, 2025
@kyleaoman @meeseeksmachine
Backport PR astropy#19055: Handle np.average with different shape w…
379c10f 
…eights and input array and returning sum of weights
@meeseeksmachine meeseeksmachine mentioned this pull request Dec 27, 2025
Merged
@mhvk mhvk mentioned this pull request Dec 27, 2025
Merged
1 task
pllim added a commit that referenced this pull request Dec 29, 2025
@pllim
Merge pull request #19097 from meeseeksmachine/auto-backport-of-pr-19…
5197913 
…055-on-v7.2.x

Backport PR #19055 on branch v7.2.x (Handle `np.average` with different shape weights and input array and returning sum of weights)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhvk mhvk mhvk approved these changes

Assignees

No one assigned

Labels

backport-v7.2.x on-merge: backport to v7.2.x Bug units

Projects

None yet

Milestone

v7.2.1

Development

Successfully merging this pull request may close these issues.

np.average strips units on sum of weights when weights and input have different shapes

3 participants

@kyleaoman @pllim @mhvk