Skip to content

Refactor Distribution a little (in preparation for structured ones) - #15295

Closed
mhvk wants to merge 2 commits into
astropy:mainfrom
mhvk:distribution-refactoring
Closed

Refactor Distribution a little (in preparation for structured ones)#15295
mhvk wants to merge 2 commits into
astropy:mainfrom
mhvk:distribution-refactoring

Conversation

@mhvk

@mhvk mhvk commented Sep 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR mostly adds tests of things I added while trying to ensure Distribution could handle structured dtypes. In addition, it makes initialization and viewing a bit more like Masked - which also helps supporting structured dtypes, but here has no net effect (the tests pass without it). I put both these additions in a separate PR to make reviewing the next one easier.

  • 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.

mhvk added 2 commits September 8, 2023 16:24
@mhvk
Test that Distribution can deal with advanced indices
7b80926
@mhvk
Get distribution class in view rather than reinitialize.
ccf974d 
As part of this, define _get_distribution_cls in analogy
to what is done in Masked._get_masked_cls. This is used
both in the initializer and in .view().
@mhvk mhvk added this to the v6.0 milestone Sep 8, 2023
@github-actions

github-actions Bot commented Sep 8, 2023

Copy link
Copy Markdown
Contributor

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?
  • Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
  • 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.

@mhvk mhvk mentioned this pull request Sep 8, 2023
Closed
1 task done
@mhvk
mhvk requested a review from eteq September 8, 2023 15:18
@mhvk mhvk mentioned this pull request Sep 11, 2023
Merged
1 task done
@mhvk

mhvk commented Sep 16, 2023

Copy link
Copy Markdown
Contributor Author

#15304, which includes this, was reviewed as a whole and will get merged, so am closing this one.

@mhvk mhvk closed this Sep 16, 2023
@mhvk
mhvk deleted the distribution-refactoring branch February 17, 2025 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eteq eteq Awaiting requested review from eteq

Assignees

No one assigned

Labels

no-changelog-entry-needed Refactoring uncertainty

Projects

None yet

Milestone

v6.0.0

Development

Successfully merging this pull request may close these issues.

1 participant

@mhvk