MNT: Run PTH flake test in prep for supporting pathlib (coordinates)#16932
MNT: Run PTH flake test in prep for supporting pathlib (coordinates)#16932neutrinoceros wants to merge 1 commit intoastropy:mainfrom
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 |
23013d1 to
9541f7d
Compare
There was a problem hiding this comment.
I tried running this script to ensure the updates don't cause any changes in the data files it produces, but I was unable to install starlink-pyast. That package has not been updated for years, so it might not be possible to run this script with current versions of our dependencies at all. This script was used for generating some data files for tests, so it requiring some specific (and old) set of dependencies to run could be acceptable, but we should first find out what that set of dependencies is and avoid making any changes to this file until we know how to run it.
There was a problem hiding this comment.
I could figure out how to install starlink-pyast and added PEP 723 metadata to the script for the next reader.
At the moment it seems that the script is indeed broken, but not by my patch: it's already broken on main. Before I dig any further, do you think it's worth fixing or should we instead remove this (clearly ?) unused script ?
There was a problem hiding this comment.
The purpose of this script is to allow replicating some of our test data. Not being able to run it is a real problem. If we can't get this script to work then we might have to replace the data that it generated.
Getting the script to work with current astropy dependencies would be the best outcome, but if some older versions are required then that can be acceptable, as long as they are adequately documented.
There was a problem hiding this comment.
I see, thanks for clarifying this. I'll spend some more time trying to fix this script or find a way to make it work with an older version if I have to.
There was a problem hiding this comment.
Have you verified that this script still works as intended?
There was a problem hiding this comment.
I actually didn't realize it was a script and might not be tested. I'll give it a try.
There was a problem hiding this comment.
Also could not run this on main: check_output(["rv", "rv.input"]) is failing with
FileNotFoundError: [Errno 2] No such file or directory: 'rv'
From the comment it seems that rv is supposed to be an entry point in Starlink, but I don't know where to install that from.
There was a problem hiding this comment.
There is a comment at the top of the file with instructions, but I haven't tried following them.
There was a problem hiding this comment.
Unfortunately these instructions do not explain where the rv command is to be found. It just says that it's needed. It looks like it was added in #10185 so let's ask @astrofrog about it.
There was a problem hiding this comment.
Still waiting on @astrofrog's reply here. In the meantime, let's bump the milestone on this one.
3fc29e5 to
aaddb6d
Compare
8a11a6d to
622cfd5
Compare
622cfd5 to
16007f0
Compare
| f" {row['obslat'].to_string('deg', sep=' ')}\n" | ||
| rv_input = Path("rv.input") | ||
| rv_input.write_text( | ||
| "".join( |
There was a problem hiding this comment.
do these need a join?
String joining happens automatically if the strings are on consecutive lines.
There was a problem hiding this comment.
thanks. Indeed there was room for simplification here
2eedd45 to
7f535c8
Compare
|
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. |
Co-authored-by: Mridul Seth <git@mriduls.com>
7f535c8 to
caece78
Compare
|
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. |
|
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. |
Description
Extracted from #16060. Ref #16924