TST: simplify `jplephem` test case for newer versions by neutrinoceros · Pull Request #18331 · astropy/astropy

neutrinoceros · 2025-06-23T10:51:22Z

Description

This test failed on weekly cron, presumably because jplephem 0.23 just came out (hard to be certain without a changelog)

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 · 2025-06-23T10:51:45Z

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 · 2025-06-23T10:51:48Z

👋 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 · 2025-06-23T11:36:26Z

Failure is unrelated. It's still blocking since the failing job is required for merging, but I'll undraft this any way since there's nothing more to be done here.
ref #18333

neutrinoceros · 2025-06-23T12:55:01Z

I just pushed a temporary patch to pin scipy on windows, working around #18333. Despite the fact that resolving orthogonal issues in a single PR isn't ideal, it's still the easiest (and to me, only) option available to unblock everything since both issues are mutually blocking: we need to fix both to re-enable PR merging.

pllim · 2025-06-23T16:13:54Z

___________________ test_ephemeris_local_file_not_ephemeris ____________________

    @pytest.mark.skipif(not HAS_JPLEPHEM, reason="requires jplephem")
    def test_ephemeris_local_file_not_ephemeris():
        # NOTE: This test currently leaves the file open (ResourceWarning).
        # To fix this issue, an upstream fix is required in jplephem
        # package.
>       with pytest.warns(ResourceWarning), pytest.raises(ValueError, match="^file starts"):
E       Failed: DID NOT WARN. No warnings of type (<class 'ResourceWarning'>,) were emitted.
E        Emitted warnings: [].

astropy/coordinates/tests/test_solar_system.py:403: Failed

eerovaher · 2025-06-23T19:54:02Z

+
+        return wrapped
+else:
+    # jplephem<=2.23 leaves the file open (a ResourceWarning is emitted)


Suggested change

# jplephem<=2.23 leaves the file open (a ResourceWarning is emitted)

# jplephem<=2.22 leaves the file open (a ResourceWarning is emitted)

or

Suggested change

# jplephem<=2.23 leaves the file open (a ResourceWarning is emitted)

# jplephem<2.23 leaves the file open (a ResourceWarning is emitted)

eerovaher · 2025-06-23T19:56:07Z

+    filterwarnings = pytest.mark.filterwarnings("ignore", category=ResourceWarning)
+
+
+@filterwarnings


Suggested change

@filterwarnings

@pytest.mark.filterwarnings("error" if minversion("jplephem", "2.23") else "ignore", category=ResourceWarning)

oh, that's much better, thanks !

… CI-breaking change in v1.16.0

eerovaher

I can approve the changes to coordinates. I can't help with Windows-specific problems, but pinning scipy on Windows seems like a sensible temporary solution to unblock CI.

neutrinoceros · 2025-06-23T21:45:53Z

@pllim, can you weigh in on the second commit ?

pllim · 2025-06-23T21:53:06Z

It is in my queue but given that I am running the coordination meeting, not sure when I can get to this. Is this urgent?

neutrinoceros · 2025-06-23T22:03:34Z

Kinda, it's about unblocking CI for every other PR

pllim

Left comments. Thanks!

pllim · 2025-06-23T22:15:21Z

-    # To fix this issue, an upstream fix is required in jplephem
-    # package.
-    with pytest.warns(ResourceWarning), pytest.raises(ValueError, match="^file starts"):
+    with pytest.raises(ValueError, match="^file starts"):


I would have done the version check inside the test function; keep the existing pytest.warns for not minversion("jplephem", "2.23") but nullcontext otherwise.

that is exactly what I did originally. What you're seeing now is after @eerovaher's review

pytest.warns suggests that the ResourceWarning is something that we want to occur, which is not the case. The warning is instead a nuisance caused by a known bug in a dependency that has been finally fixed, and as such should be explicitly ignored with filterwarnings. The fact that pytest.warns also prevents warnings from causing failures in our tests is a side-effect.

pllim · 2025-06-23T22:16:39Z

 # Recommended run-time dependencies to enable a lot of functionality within Astropy.
 recommended = [
    "scipy>=1.9.2",
+    "scipy<1.16 ; platform_system=='Windows'", # temp, https://github.com/astropy/astropy/issues/18333


Can we address this as separate PR? I haven't had a chance to look at the logs yet and maybe we don't need this after all.

unfortunately I don't think we can, unless you're able to override CI requirements, because both issues are separately breaking the Windows job, which is required.

pllim

To unblock things, let's merge. But I want to revisit the Windows one. Thanks!

lumberbot-app · 2025-06-24T05:39:59Z

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

git checkout v7.1.x
git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 5d0bcab5036724a0e7a95df042719aa3e1885017

You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #18331: TST: simplify `jplephem` test case for newer versions'

Push to a named branch:

git push YOURFORK v7.1.x:auto-backport-of-pr-18331-on-v7.1.x

Create a PR against branch v7.1.x, I would have named this PR:

"Backport PR #18331 on branch v7.1.x (TST: simplify jplephem test case for newer versions)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

pllim · 2025-06-24T05:41:37Z

@neutrinoceros are you able to do manual backport? Thanks! 🙏

…test case for newer versions) TST: simplify `jplephem` test case for newer versions

neutrinoceros · 2025-06-24T10:26:55Z

manual backport at #18335

…test case for newer versions) TST: simplify `jplephem` test case for newer versions

pllim · 2025-06-25T05:58:15Z

Thanks!

Backport PR #18331 on branch v7.1.x (TST: simplify `jplephem` test case for newer versions)

neutrinoceros added this to the v7.1.1 milestone Jun 23, 2025

neutrinoceros added testing Bug Extra CI Run cron CI as part of PR backport-v7.1.x labels Jun 23, 2025

github-actions Bot added the coordinates label Jun 23, 2025

neutrinoceros added the no-changelog-entry-needed label Jun 23, 2025

neutrinoceros marked this pull request as ready for review June 23, 2025 11:36

This was referenced Jun 23, 2025

TST: decoupling stable CI and incompatibilities discovery (let's use lock files) #17596

Open

BUG: a failing modeling test on windows #18333

Open

eerovaher reviewed Jun 23, 2025

View reviewed changes

Comment thread astropy/coordinates/tests/test_solar_system.py Resolved

neutrinoceros force-pushed the coordinates/tst/fix-jplephem-test branch from 4c0fe8e to 4567384 Compare June 23, 2025 18:44

neutrinoceros marked this pull request as draft June 23, 2025 18:49

neutrinoceros force-pushed the coordinates/tst/fix-jplephem-test branch from 4567384 to 52150f9 Compare June 23, 2025 19:01

neutrinoceros marked this pull request as ready for review June 23, 2025 19:07

eerovaher reviewed Jun 23, 2025

View reviewed changes

neutrinoceros added 2 commits June 23, 2025 22:05

TST: simplify jplephem test case for newer versions

5616b5e

TMP: set an upper limit to scipy's version on windows to workaround a…

0c2eaad

… CI-breaking change in v1.16.0

neutrinoceros force-pushed the coordinates/tst/fix-jplephem-test branch from 52150f9 to 0c2eaad Compare June 23, 2025 20:05

eerovaher approved these changes Jun 23, 2025

View reviewed changes

pllim reviewed Jun 23, 2025

View reviewed changes

pllim approved these changes Jun 24, 2025

View reviewed changes

pllim merged commit 5d0bcab into astropy:main Jun 24, 2025

lumberbot-app Bot added the Still Needs Manual Backport label Jun 24, 2025

neutrinoceros deleted the coordinates/tst/fix-jplephem-test branch June 24, 2025 06:01

neutrinoceros mentioned this pull request Jun 24, 2025

Backport PR #18331 on branch v7.1.x (TST: simplify jplephem test case for newer versions) #18335

Merged

1 task

neutrinoceros pushed a commit to neutrinoceros/astropy that referenced this pull request Jun 24, 2025

Backport PR astropy#18331 on branch v7.1.x (TST: simplify jplephem …

6070ab5

…test case for newer versions) TST: simplify `jplephem` test case for newer versions

neutrinoceros removed the Still Needs Manual Backport label Jun 24, 2025

pllim added a commit to neutrinoceros/astropy that referenced this pull request Jun 25, 2025

Backport PR astropy#18331 on branch v7.1.x (TST: simplify jplephem …

e1b7efb

…test case for newer versions) TST: simplify `jplephem` test case for newer versions

pllim added a commit that referenced this pull request Jun 26, 2025

Merge pull request #18335 from neutrinoceros/bp/7.1.x/18331

9e288fa

Backport PR #18331 on branch v7.1.x (TST: simplify `jplephem` test case for newer versions)

pllim mentioned this pull request Jul 2, 2025

TST: Add Sersic2D to NON_FINITE_LM_MODELS #18381

Merged

1 task

pllim mentioned this pull request Sep 2, 2025

Rebuild for python 3.14 conda-forge/astropy-feedstock#163

Merged

	# jplephem<=2.23 leaves the file open (a ResourceWarning is emitted)
	# jplephem<=2.22 leaves the file open (a ResourceWarning is emitted)

	# jplephem<=2.23 leaves the file open (a ResourceWarning is emitted)
	# jplephem<2.23 leaves the file open (a ResourceWarning is emitted)

		filterwarnings = pytest.mark.filterwarnings("ignore", category=ResourceWarning)


		@filterwarnings

	@filterwarnings
	@pytest.mark.filterwarnings("error" if minversion("jplephem", "2.23") else "ignore", category=ResourceWarning)

Conversation

neutrinoceros commented Jun 23, 2025

Description

Uh oh!

github-actions Bot commented Jun 23, 2025

Uh oh!

github-actions Bot commented Jun 23, 2025

Uh oh!

neutrinoceros commented Jun 23, 2025

Uh oh!

neutrinoceros commented Jun 23, 2025

Uh oh!

pllim commented Jun 23, 2025

Uh oh!

Uh oh!

eerovaher Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

eerovaher Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

eerovaher left a comment

Choose a reason for hiding this comment

Uh oh!

neutrinoceros commented Jun 23, 2025

Uh oh!

pllim commented Jun 23, 2025

Uh oh!

neutrinoceros commented Jun 23, 2025

Uh oh!

pllim left a comment

Choose a reason for hiding this comment

Uh oh!

pllim Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

eerovaher Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

pllim Jun 23, 2025

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pllim left a comment

Choose a reason for hiding this comment

Uh oh!

lumberbot-app Bot commented Jun 24, 2025

Uh oh!

pllim commented Jun 24, 2025

Uh oh!

neutrinoceros commented Jun 24, 2025

Uh oh!

pllim commented Jun 25, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

neutrinoceros Jun 23, 2025 •

edited

Loading