-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
io.misc.asdf: skip modeling doctest that requires removed features in asdf (v5.3.x) #14797
Conversation
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.
|
👋 Thank you for your draft pull request! Do you know that you can use |
It appears I do not have the ability to set labels so could not follow |
Close-reopen for the |
@@ -942,7 +942,7 @@ format. This can be useful in many contexts, one of which is the implementation | |||
|
|||
Serializing a model to disk is possible by assigning the object to ``AsdfFile.tree``: | |||
|
|||
.. doctest-requires:: asdf | |||
.. doctest-requires:: asdf<3.0.0.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is supported. Would be nice if it does though. Let's see...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modeling passed: https://github.com/astropy/astropy/actions/runs/4929120049/jobs/8808352373?pr=14797#step:10:1646
However coodinates index.rst failed:
346 .. doctest-remote-data::
347
348 >>> EarthLocation.of_address('1002 Holy Grail Court, St. Louis, MO') # doctest: +FLOAT_CMP
349 <EarthLocation (-26769.86528679, -4997007.71191864, 3950273.57633915) m>
350 >>> EarthLocation.of_address('Danbury, CT') # doctest: +FLOAT_CMP
Expected:
<EarthLocation (1362610.66896362, -4590755.48088484, 4198817.69912853) m>
Got:
<EarthLocation (1364606.6451165, -4593292.9428273, 4195415.93695139) m>
/home/runner/work/astropy/astropy/docs/coordinates/index.rst:350: DocTestFailure
Is this a known issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing 5.2.2 I get the 'Got' result:
In [22]: import astropy.coordinates
In [23]: astropy.coordinates.EarthLocation.of_address('Danbury, CT')
Out[23]: <EarthLocation (1364606.6451165, -4593292.9428273, 4195415.93695139) m>
In [24]: astropy.__version__
Out[24]: '5.2.2'
Did someone move Danbury?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The EarthLocation
look-up been flaky lately, just ignore it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure(I'm afk), is it not listed in the changelog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't comb through the change log. Maybe I should. The README didn't mention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind opening a quick PR then? We should be more diligent with requiring the readme updates with the enhancements, but afaik we're not insistent enough
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@braingram , are you able to open similar PRs to v5.2.x and v5.0.x ? 🙏 |
@pllim I opened similar PRs against v5.0.x and v5.2.x. As I am unable to set labels they will require a similar set of steps as @WilliamJamieson performed on this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Manual Backport PR #14797 to v5.0.x
Manual Backport PR #14797 to v5.2.x
Description
2 modeling doctests (like the following):
astropy/docs/modeling/models.rst
Lines 947 to 952 in eec0697
should have been skipped in #14516
It has since been updated in main: #14668
and requires a manual backport to fix this failing test as described:
#14795