Skip to content

MNT: Run PTH flake test in prep for supporting pathlib (time)#16923

Merged
pllim merged 1 commit intoastropy:mainfrom
neutrinoceros:time/rfc/pth_checks
Sep 6, 2024
Merged

MNT: Run PTH flake test in prep for supporting pathlib (time)#16923
pllim merged 1 commit intoastropy:mainfrom
neutrinoceros:time/rfc/pth_checks

Conversation

@neutrinoceros
Copy link
Contributor

@neutrinoceros neutrinoceros commented Sep 3, 2024 •
edited
Loading

Description

Extracted from #16060

@taldcroft already approved this bit in #16060 (review)

  • 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
Copy link
Contributor

github-actions bot commented Sep 3, 2024

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-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2024

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@neutrinoceros neutrinoceros force-pushed the time/rfc/pth_checks branch 2 times, most recently from abf8a7f to 34c0b33 Compare September 3, 2024 08:22
@neutrinoceros neutrinoceros mentioned this pull request Sep 3, 2024
Closed
@pllim pllim added this to the v7.0.0 milestone Sep 3, 2024
@neutrinoceros neutrinoceros force-pushed the time/rfc/pth_checks branch 2 times, most recently from 209c130 to 2cecd81 Compare September 3, 2024 16:42
@pllim pllim mentioned this pull request Sep 3, 2024
Closed
@neutrinoceros neutrinoceros marked this pull request as ready for review September 3, 2024 16:53
@neutrinoceros
Copy link
Contributor Author

neutrinoceros commented Sep 3, 2024

Note that labels Docs, testing and utils.iers can be removed (they were due to commit from the branch this was previously based off)

pllim
pllim reviewed Sep 3, 2024
View reviewed changes
pllim
pllim reviewed Sep 3, 2024
View reviewed changes
astropy/time/tests/test_update_leap_seconds.py Outdated

with pytest.warns(AstropyWarning, match="ValueError.*did not find expiration"):
update_leap_seconds([bad_file])
# FIXME: update_leap_seconds should be able to handle Path objects
Copy link
Member

@pllim pllim Sep 3, 2024

Choose a reason for hiding this comment

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

If this is not an issue already, maybe we should open a GitHub issue and link that here?

Copy link
Contributor Author

@neutrinoceros neutrinoceros Sep 3, 2024

Choose a reason for hiding this comment

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

yes. I just took a very quick look and it's indeed not trivial to fix, so an issue seems appropriate. I can open it tomorrow (unless you want to beat me to it)

Copy link
Member

@pllim pllim Sep 3, 2024

Choose a reason for hiding this comment

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

You can probably provide better details. I can wait. Thanks! 😺

Copy link
Contributor Author

@neutrinoceros neutrinoceros Sep 4, 2024

Choose a reason for hiding this comment

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

done in #16929 (and linked in a comment here)

Copy link
Contributor Author

@neutrinoceros neutrinoceros Sep 5, 2024

Choose a reason for hiding this comment

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

moving this PR to draft so #16931 can be prioritized. This comment can be removed once #16931 in merged.

Copy link
Member

@pllim pllim Sep 5, 2024

Choose a reason for hiding this comment

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

The linked PR is merged. FYI.

Copy link
Contributor Author

@neutrinoceros neutrinoceros Sep 6, 2024

Choose a reason for hiding this comment

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

thanks for reminding me ! rebased, and soon-to-be undrafted

@neutrinoceros neutrinoceros requested a review from a team as a code owner September 3, 2024 19:48
@neutrinoceros neutrinoceros mentioned this pull request Sep 4, 2024
Closed
@neutrinoceros neutrinoceros marked this pull request as draft September 5, 2024 09:15
@neutrinoceros neutrinoceros changed the title MNT: Run PTH flake test in prep for supporting pathlib (time) MNT: Run PTH flake test in prep for supporting pathlib (time) (⏰ wait for #16931) Sep 5, 2024
@neutrinoceros @MridulS
MNT: Run PTH flake test in prep for supporting pathlib (time)
d9d1145 
Co-authored-by: Mridul Seth <git@mriduls.com>
@neutrinoceros neutrinoceros changed the title MNT: Run PTH flake test in prep for supporting pathlib (time) (⏰ wait for #16931) MNT: Run PTH flake test in prep for supporting pathlib (time) Sep 6, 2024
@neutrinoceros neutrinoceros marked this pull request as ready for review September 6, 2024 07:00
mhvk
mhvk approved these changes Sep 6, 2024
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.

Looks good to me too. Since already indirectly approved by Tom, I'll go ahead and merge.

@mhvk
Copy link
Contributor

mhvk commented Sep 6, 2024

Actually, I cannot merge since the aarch64 failed -- operation cancelled -- though it does not seem related...

@neutrinoceros
Copy link
Contributor Author

neutrinoceros commented Sep 6, 2024

yes it's an unrelated timeout which we've been seeing a lot lately

@pllim
Copy link
Member

pllim commented Sep 6, 2024

I cannot merge since the aarch64 failed

Merge should not have been disabled because of that job. It is not one of the required jobs for branch protection. 🤔

@pllim pllim merged commit ae7d72f into astropy:main Sep 6, 2024
@pllim
Copy link
Member

pllim commented Sep 6, 2024 •
edited
Loading

For completeness, the timeout is a separate issue #16565

Thanks, all!

@neutrinoceros neutrinoceros deleted the time/rfc/pth_checks branch September 6, 2024 15:29
@neutrinoceros neutrinoceros mentioned this pull request Jun 10, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pllim pllim pllim left review comments

@mhvk mhvk mhvk approved these changes

@WilliamJamieson WilliamJamieson Awaiting requested review from WilliamJamieson WilliamJamieson is a code owner

@nstarman nstarman Awaiting requested review from nstarman nstarman is a code owner

@taldcroft taldcroft Awaiting requested review from taldcroft taldcroft is a code owner

Assignees

No one assigned

Labels

dev-automation Extra CI Run cron CI as part of PR no-changelog-entry-needed time

Projects

None yet

Milestone

v7.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@neutrinoceros @mhvk @pllim