Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Doc implement reredirects#19456

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
anntzer merged 4 commits intomatplotlib:masterfromjklymak:doc-impliment-reredirects
Feb 25, 2021

Conversation

jklymak
Copy link
Member

@jklymakjklymak commentedFeb 4, 2021•
edited
Loading

PR Summary

UPDATE:

This is based off#19440 just because that is a good example where some existing files were deleted.

This now uses a vendored versionhttps://github.com/anntzer/sphinx-redirectfrom@anntzer put together. In#19440doc/api/backend_qt4agg_api.rst was removed, and the info is now indoc/api/backend_qt_api.rst sodoc/api/backend_qt4agg_api.rst now has aredirect-from directive:

.. redirect-from:: /api/backend_qt4agg_api

When the build is made this makesbuild/html/backend_qt4agg_api.html which has a refresh:

<html><head><metahttp-equiv="refresh"content="0; url=backend_qt_api.html"><linkrel="canonical"href="backend_qt_api.html"></head></html>

so the old link still exists.

(probably not super necessary for this example, but in general should be quite useful).

I think there are in general some major disadvantages compared with thereredirect approach. Namely that moving whole directories will require each file in the directory getting this directive to its new location. OTOH, that is pretty easy to script up if someone needs to do a bulk move.

  • add instructions in documentation guide about how to do this

PR Checklist

  • Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (runflake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
  • Conforms to Matplotlib style conventions (installflake8-docstrings and runflake8 --docstring-convention=all).
  • New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).

@jklymakjklymakforce-pushed thedoc-impliment-reredirects branch 3 times, most recently from74331a4 toabeef9dCompareFebruary 5, 2021 05:01
@QuLogic
Copy link
Member

The placeholders cause the build to fail:

checking consistency... /home/circleci/project/doc/api/backend_gtk3agg_api.rst: WARNING: document isn't included in any toctree/home/circleci/project/doc/api/backend_gtk3cairo_api.rst: WARNING: document isn't included in any toctree/home/circleci/project/doc/api/backend_qt4agg_api.rst: WARNING: document isn't included in any toctree/home/circleci/project/doc/api/backend_qt4cairo_api.rst: WARNING: document isn't included in any toctree/home/circleci/project/doc/api/backend_qt5agg_api.rst: WARNING: document isn't included in any toctree/home/circleci/project/doc/api/backend_qt5cairo_api.rst: WARNING: document isn't included in any toctree/home/circleci/project/doc/api/backend_tkagg_api.rst: WARNING: document isn't included in any toctree/home/circleci/project/doc/api/backend_wxagg_api.rst: WARNING: document isn't included in any toctreedone

@jklymakjklymak marked this pull request as draftFebruary 5, 2021 11:49
@jklymak
Copy link
MemberAuthor

The placeholders cause the build to fail:

Yep. Those can be orphaned. Looks like the links need to be relative to the directory that they are being made in as well (despite what the docs say). Making them absolute to root probably works too, but doesn't work in the artifacts html...

@jklymak
Copy link
MemberAuthor

Well, that works. I'm not a huge fan to be honest, and almost wish we could just have a new sphinx directive. Is that easy?:redirect: ./newpage would be a lot easier than orphaning and adding to theconf.py.

@jklymak
Copy link
MemberAuthor

@anntzer
Copy link
Contributor

I'll have a look at the directive possibility.

@jklymak
Copy link
MemberAuthor

Cool, it should be pretty straightforward. I may look at it as well, but probably a bit of learning curve. For reference, all the reredirects does is change the html to:

<html><head><metahttp-equiv="refresh"content="0; url=${to_uri}"></head></html>

It might be nice to spice that up a bit (add a canonical?) but I think even as-is search engines will recognize that this is not supposed to be indexed.

@anntzer
Copy link
Contributor

@jklymak Seehttps://github.com/anntzer/sphinx-redirectfrom (quick hack, barely tested, unlikely to eat your dog though). I went for what I think is a slightly nicer approach of not actually requiring to keep the old files in the source tree and having a.. redirect-from:: /path/to/old/src in thenew source tree.

@jklymak
Copy link
MemberAuthor

Nice, I think I agree thats better, because if we delete the new target, it would be nice to know we have to redirect again. I'll play with that over the w/e. Thanks a lot!

@jklymakjklymakforce-pushed thedoc-impliment-reredirects branch 4 times, most recently from8087a9d to3af306cCompareFebruary 6, 2021 18:46
@jklymakjklymak added the status: needs comment/discussionneeds consensus on next step labelFeb 6, 2021
@jklymak
Copy link
MemberAuthor

@jklymakjklymakforce-pushed thedoc-impliment-reredirects branch 4 times, most recently frombeec731 to9289f61CompareFebruary 7, 2021 05:02
@anntzer
Copy link
Contributor

anntzer commentedFeb 7, 2021•
edited
Loading

Namely that moving whole directories will require each file in the directory getting this directive to its new location.

I guess you could support both.. redirect-from:: and a pre-set list of redirects in conf.py for glob redirects (that would just pre-populate theredirects mapping).

@jklymakjklymakforce-pushed thedoc-impliment-reredirects branch from9289f61 to4a205dbCompareFebruary 7, 2021 16:11
@jklymak
Copy link
MemberAuthor

Namely that moving whole directories will require each file in the directory getting this directive to its new location.

I guess you could support both.. redirect-from:: and a pre-set list of redirects in conf.py for glob redirects (that would just pre-populate theredirects mapping).

Sure, well we could cross that bridge when we come to it. For now we have a pretty easy way to redirect if we move, so I think this can wait.

@jklymakjklymak marked this pull request as ready for reviewFebruary 7, 2021 16:39
@jklymakjklymak added this to thev3.4.0 milestoneFeb 7, 2021
@QuLogic
Copy link
Member

@jklymakjklymak added the status: needs comment/discussion label4 days ago

In the meeting, or is it okay to merge now?

@jklymakjklymak removed the status: needs comment/discussionneeds consensus on next step labelFeb 11, 2021
@jklymak
Copy link
MemberAuthor

This is fine from my end. I think folks have had lots of time to voice concerns. Plus its not an irrevocable change if we decide this is a bad way to go.

@jklymak
Copy link
MemberAuthor

@timhoffm@story645@dstansby@brunobeltran This make sense to all of you?

@jklymak
Copy link
MemberAuthor

Need to add warning for an overwrite....

@jklymakjklymak marked this pull request as draftFebruary 11, 2021 21:23
@jklymakjklymakforce-pushed thedoc-impliment-reredirects branch 3 times, most recently from7049b88 tocf7f0bbCompareFebruary 11, 2021 22:08
ENH: add redirect_from sphinx extensionCo-authored-by: Antony Lee <anntzer.lee@gmail.com>Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
@jklymakjklymakforce-pushed thedoc-impliment-reredirects branch fromcf7f0bb to7e4a922CompareFebruary 11, 2021 23:43
@jklymakjklymak marked this pull request as ready for reviewFebruary 12, 2021 00:23
@jklymakjklymakforce-pushed thedoc-impliment-reredirects branch froma00a2bf to58f495aCompareFebruary 15, 2021 21:30
@jklymakjklymakforce-pushed thedoc-impliment-reredirects branch from58f495a to47145d1CompareFebruary 16, 2021 00:43
@jklymak
Copy link
MemberAuthor

.. added a redirect from/users/customizing to/tutorials/introductory/customizing as that was moved in#8545 (as discussed on gitter)

@tacaswell
Copy link
Member

Did we mean to take outexamples/subplots_axes_and_figures/subplot_toolbar.py?

@jklymak
Copy link
MemberAuthor

Did we mean to take outexamples/subplots_axes_and_figures/subplot_toolbar.py?

Ooops! No, I mean we could ;-) It crashes on my machine when I do the doc build.

jklymakand others added2 commitsFebruary 16, 2021 20:14
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
@jklymak
Copy link
MemberAuthor

I think this is still in good shape? I wouldn't mind it being merged so we can start doing more manual redirects

@anntzeranntzer merged commit9bfd631 intomatplotlib:masterFeb 25, 2021
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestFeb 25, 2021
tacaswell added a commit that referenced this pull requestFeb 25, 2021
…456-on-v3.4.xBackport PR#19456 on branch v3.4.x (Doc implement reredirects)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@QuLogicQuLogicQuLogic approved these changes

@anntzeranntzeranntzer approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.

5 participants
@jklymak@QuLogic@anntzer@tacaswell@timhoffm
[8]ページ先頭
©2009-2025 Movatter.jp