WebbPSF to STPSF #951
WebbPSF to STPSF #951
Conversation
|
Hello @BradleySappington, Thank you for updating ! There are no PEP8 issues in this Pull Request. Comment last updated at 2024-12-19 15:30:54 UTC |
|
Read the docs redirect. Is there a way to make that global across all pages? Would we need to put that redirect text on each of the individual RST pages? Thanks so much for all the work on this -- so many little details to keep track of! |
|
@mperrin - I don't actually think this readthedocs redirect is needed. I created a domain redirect through readthedocs admin settings and have successfully tested (I just have it deactivated currently) |
|
superb, I'm glad to hear that there's a way to do a domain redirect, and that you've already figured it out! :-) |
|
I removed some of the workflows as we are now importing everything from stpsf (including tests) they are failing here. No sense in spending time making the workflows functional to archive the repo in a few months so I removed them. Will not merge until confirmed everyone is comfortable with these changes. |
|
|
||
| If you would like to receive email announcements of future versions, please contact Marshall Perrin, or visit `maillist.stsci.edu <https://maillist.stsci.edu/scripts/wa.exe?SUBED1=webbpsf-users&A=1>` to subscribe yourself to the "[email protected]" list. | ||
| .. meta:: | ||
| :http-equiv=Refresh: 0; url='https://stpsf.readthedocs.io/en/latest/' |
There was a problem hiding this comment.
Did you decide this redirect was needed, or were you going to use the domain redirect option in readthedocs that you mentioned? Either way is fine with me - I just want to make sure we all understand what the plan is and how it's going to work.
Relatedly, I see the docs build was left in the CI tests. That makes me think that the plan is not to use a domain redirect in readthedocs, since if we're just redirecting the whole domain, why do we need to test that these docs build?
There was a problem hiding this comment.
I'd like to stick with the redirect through readthedocs. I had made this change initially before realizing I could do it on read the docs. I chose not to change it back because I liked the code showing the intention of not using this documentation any more, however did not feel it necessary to delete the entire documentation base. If you'd like me to put back please advise.
| py{310,311,312}-{poppydev,pysiafdev,astropydev,latest,stable}-test | ||
| py310-legacy-test | ||
| py{310,311,312}-{poppydev,pysiafdev}-cov | ||
| envlist = docbuild |
There was a problem hiding this comment.
This doesn't seem right to me to remove all test environments apart from the docs.
We're still going to have the webbpsf package as a wrapper that imports stpsf, and being able to do import webbpsf and have existing code run is a KEY PART of the intended functionality here.
Therefore there needs to be a test suite that verifies that we can indeed import webbpsf and run calculations.
So please let's keep at least one test environment that runs the existing full test suite of webbpsf test functions. I don't think this needs to have all the different instances in the test matrix -- those will be covered by the stpsf test suite -- but there should be enough here to test that the import wrapper is working as intended.
There was a problem hiding this comment.
Context for removing all the test environments:
Because the entire repository is replaced with stpsf, the tests that are running are stpsf tests. This wasn't playing nicely with our CI as all tests labelled webbpsf no longer existed. As this was intended to be the final release of WebbPSF, I decided to manually verify that the init change worked properly (as these automated tests will not need to run in the future because development is stopping in this repository). I think that if we all take a look and are comfortable with testing the build manually, that suffices. If this makes anyone uncomfortable I can spend time trying to get the CI functional, however I felt the benefits of that time investment were minimal.
|
|
||
| if not _warned: | ||
| warnings.warn( |
There was a problem hiding this comment.
As discussed in slack, I'm not sure a multi-line warning with lots of !!!!! is the best choice at this time. Suggest making it shorter, and making a webpage in the docs for more information on the name migration.
Perhaps like:
"""
This message is for information only and WebbPSF will continue to function as normal.
The WebbPSF library has been moved/renamed to STPSF. Please see [READTHEDOCS_PAGE_TBD] for more information.
WebbPSF is now an alias of STPSF and is running code from the STPSF library.
"""
And then that webpage, some new child page within the stpsf docs, can give more details and example code, rather than trying to pack too much into a warning message.
There was a problem hiding this comment.
Replacing exactly as suggested here with a TODO until we have a proper page with instructions set up (stpsf 2.0.1)
transition webbpsf to stpsf alias