Skip to content
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

Fix URLs redirects #187

Merged
merged 2 commits into from
Nov 7, 2024
Merged

Fix URLs redirects #187

merged 2 commits into from
Nov 7, 2024

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Nov 6, 2024

Testing the URL breakages in #185

@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 6, 2024

@efiring while having accurate URLs in the docs is nice I'm tempted to skip the URL check for now b/c a correct fix should be in the upstream matlab docs, right?

@ocefpaf ocefpaf mentioned this pull request Nov 6, 2024
@efiring
Copy link
Member

efiring commented Nov 6, 2024

@ocefpaf I think the corrections you have in gsw.rst and __init__.py are where they should be. for _wrapped_ufuncs the corrections should be made by adding entries to subs in fix_wrapped_ufuncs.py. It looks like maybe only one additional entry is needed there.

Getting corrections upstream is a reasonable goal for the future, but not something I think we should try right now.

@efiring
Copy link
Member

efiring commented Nov 6, 2024

Also, I don't understand why some of the teos-10 urls are failing in the test. Both the original http: the new https versions work in my browser. The test is apparently not allowing the redirect? In any case, fixing them via the sub mechanism makes sense, as far as I can see.

@ocefpaf
Copy link
Member Author

ocefpaf commented Nov 7, 2024

Yeah, the redirection works fine in the browser and sporadically here when I re-run the tests. That also failed with both the buggy micromamba and a pure pip solution. I wonder if the 403 is b/c we are hitting that URL multiple times (it is everywhere in the docs).

@ocefpaf ocefpaf marked this pull request as ready for review November 7, 2024 10:26
@ocefpaf ocefpaf requested a review from efiring November 7, 2024 10:26
Copy link
Member

@efiring efiring left a comment

Choose a reason for hiding this comment

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

Good, thank you.

@efiring efiring merged commit aa1ebe7 into TEOS-10:main Nov 7, 2024
32 checks passed
@ocefpaf ocefpaf deleted the fix_redirects branch November 8, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants