Skip to content

Enable using array of obstime for scalar coordinate value, fixes #12965 - #12968

Closed
maxnoe wants to merge 1 commit into
astropy:mainfrom
maxnoe:altaz_obstime_array
Closed

Enable using array of obstime for scalar coordinate value, fixes #12965#12968
maxnoe wants to merge 1 commit into
astropy:mainfrom
maxnoe:altaz_obstime_array

Conversation

@maxnoe

@maxnoe maxnoe commented Mar 18, 2022 •
edited by pllim
Loading

Copy link
Copy Markdown
Member

Description

Determine common broadcast shape in FrameAttribute.__get__ to fix #12965

Fixes #12965

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

  • 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 "When to rebase and squash commits".
  • 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.
  • 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 astropy-bot check might be missing; 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.

@pllim

This comment was marked as resolved.

Sign in to view
@pllim pllim added this to the v5.0.3 milestone Mar 18, 2022
@maxnoe

maxnoe commented Mar 18, 2022 •
edited
Loading

Copy link
Copy Markdown
Member Author

I now am sure that I linked the wrong one: off by 1000

@maxnoe maxnoe changed the title Enable using array of obstime for scalar coordinate value, fixes #11965 Enable using array of obstime for scalar coordinate value, fixes #12965 Mar 18, 2022
@pllim

pllim commented Mar 18, 2022

Copy link
Copy Markdown
Member

Since this is a bug fix, please add change log. Thanks!

@ayshih ayshih mentioned this pull request Mar 19, 2022
Closed
mhvk
mhvk reviewed Mar 20, 2022
View reviewed changes

@mhvk mhvk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I share @ayshih's worry that this is a bit too brute force - but it is interesting that it does work for your case (which is a good example that should really work). Maybe best to discuss best implementation in #12965

Comment thread astropy/coordinates/attributes.py Outdated
out.shape != instance_shape):

# If the shapes do not match, try broadcasting.
target_shape = np.broadcast(out, instance).shape

@mhvk mhvk Mar 20, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These days there is np.broadcast_shapes

@pllim pllim modified the milestones: v5.0.3, v5.0.4 Mar 25, 2022
@maxnoe
maxnoe force-pushed the altaz_obstime_array branch from 0dd8f51 to 320f498 Compare March 25, 2022 12:21
@pllim pllim modified the milestones: v5.0.4, v5.0.5 Mar 31, 2022
@github-actions github-actions Bot added the Close? Tell stale bot that this issue/PR is stale label Aug 23, 2022
@github-actions

github-actions Bot commented Aug 23, 2022

Copy link
Copy Markdown
Contributor

Hi humans 👋 - this pull request hasn't had any new commits for approximately 4 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock.

If you believe I commented on this pull request incorrectly, please report this here.

@github-actions

github-actions Bot commented Sep 22, 2022

Copy link
Copy Markdown
Contributor

I'm going to close this pull request as per my previous message. If you think what is being added/fixed here is still important, please remember to open an issue to keep track of it. Thanks!

If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here.

@github-actions github-actions Bot added the closed-by-bot Closed by stale bot label Sep 22, 2022
@github-actions github-actions Bot closed this Sep 22, 2022
lpsinger added a commit to lpsinger/astropy that referenced this pull request Aug 3, 2023
@lpsinger
Add regression test from astropy#12968
1766c9d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhvk mhvk mhvk left review comments

Assignees

No one assigned

Labels

Bug Close? Tell stale bot that this issue/PR is stale closed-by-bot Closed by stale bot coordinates

Projects

None yet

Milestone

v5.0.5

Development

Successfully merging this pull request may close these issues.

Transforming an AltAz coordinate generated from values of another AltAz frame does not work.

3 participants

@maxnoe @pllim @mhvk