Skip to content

Drop the out parameter cosmology's _cosmology_not_equal does not take - #20214

Open
siddartha018 wants to merge 1 commit into
astropy:mainfrom
siddartha018:docs/drop-stale-out-param
Open

siddartha018 wants to merge 1 commit into
astropy:mainfrom
siddartha018:docs/drop-stale-out-param

Conversation

@siddartha018

Copy link
Copy Markdown
Contributor

Description

Fixes #20208.

_cosmology_not_equal has the signature

def _cosmology_not_equal(cosmo1: Any, cosmo2: Any, /, *, allow_equivalent: bool = False) -> bool:

and documents an out parameter, described as "a location into which the result is stored", which is not accepted here or anywhere along the _comparison_decorator path — the wrapper forwards **kwargs to the wrapped function, which would raise on out.

format in the same section stays: it is not in the signature either, but _comparison_decorator consumes it and its own Notes say every decorated function should document it. out has no such counterpart, and cosmology_equal directly above — same decorator, same signature — has never carried the entry.

Docstring only — no signature, call site, or behaviour is touched.

Changelog

Not added: docstring only, and the function is private, so nothing user-visible changes. Per CONTRIBUTING.md, minor documentation updates do not need a fragment. Happy to add one or take the no-changelog-entry-needed label if you prefer.

@siddartha018
siddartha018 requested a review from a team as a code owner August 5, 2026 21:55
@siddartha018
siddartha018 requested a review from nstarman August 5, 2026 21:55
@github-actions

github-actions Bot commented Aug 5, 2026

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

@github-project-automation github-project-automation Bot moved this from In Progress to Reviewer approved in Cosmology, the Expansion Aug 6, 2026
@nstarman

nstarman commented Aug 6, 2026

Copy link
Copy Markdown
Member

Hm. The CI isn't running. 🤔

shape that the inputs broadcast to. If not provided or None, a
freshly-allocated array is returned.

format : bool or None or str or tuple thereof, optional keyword-only

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@nstarman what about this one?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

which one? format ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, format is mentioned but not in signature. Is that expected?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hm. Nor in cosmology_equal. Those were clearly intended plans, but I don't recollect the details.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Might as well address that also in this PR?

@pllim pllim added this to the v7.2.3 milestone Aug 6, 2026
@pllim pllim added Bug backport-v7.2.x on-merge: backport to v7.2.x backport-v8.0.x on-merge: backport to v8.0.x Docs labels Aug 6, 2026
@nstarman
nstarman self-requested a review August 7, 2026 17:37
@nstarman nstarman moved this from Reviewer approved to Review in progress in Cosmology, the Expansion Aug 7, 2026
@siddartha018
siddartha018 force-pushed the docs/drop-stale-out-param branch from 9c43063 to bd9abe9 Compare September 27, 2026 20:47

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Review in progress

Development

Successfully merging this pull request may close these issues.

_cosmology_not_equal documents an out parameter it does not take

4 participants