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

DOC: first pass at switching to pydata theme #19731

Merged
merged 1 commit into from
Jun 21, 2021

Conversation

tacaswell
Copy link
Member

@tacaswell tacaswell commented Mar 18, 2021

PR Summary

Still super busted.

Builds clean and looks mostly OK!

@tacaswell tacaswell added this to the v3.5.0 milestone Mar 18, 2021
@jklymak
Copy link
Member

jklymak commented Mar 21, 2021

Did you ever get this to build?

@tacaswell
Copy link
Member Author

It builds locally, I forgot to add the extra dependency to the requirements 🤦 .

@tacaswell
Copy link
Member Author

It at least builds!

@jklymak
Copy link
Member

jklymak commented Mar 26, 2021

Nice... https://55478-1385122-gh.circle-artifacts.com/0/doc/build/html/index.html Obviously the front page is a mess, but we are replacing anyway. The rest definitely shows promise...

@timhoffm
Copy link
Member

timhoffm commented Mar 28, 2021

Looks quite good. Some open style issues:

@story645 story645 linked an issue Jun 3, 2021 that may be closed by this pull request
@tacaswell tacaswell marked this pull request as ready for review June 18, 2021 23:13
@tacaswell
Copy link
Member Author

Made more progress! Figured out that we were fully shadowing core style sheet which is why so many things looked broken.

I have addressed the issues from @timhoffm above. It bulids clean locally,

@tacaswell
Copy link
Member Author

If this is clean and good enough, can we merge this quickly and iterate?

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

I'm fine w/ this going in. The front page looks kind of bad, but it is its own thing now, right?

@tacaswell
Copy link
Member Author

The front page looks kind of bad, but it is its own thing now, right?

There needs to be a bit more work on that, but yes.

doc/conf.py Outdated
# html_logo = 'logo.png'
html_logo = "_static/logo2.svg"
html_theme_options = {
"logo_link": "/stable/index.html",
Copy link
Member

Choose a reason for hiding this comment

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

Is that what we want on versioned pages?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not. This also makes the locally built version of the docs not self-contained (so does the third party page link, but that includes the full URL so it atleast works (assuming you have network). I'll change this.

doc/conf.py Outdated
@@ -24,16 +24,16 @@
# If your extensions are in another directory, add it here. If the directory
# is relative to the documentation root, use os.path.abspath to make it
# absolute, like shown here.
sys.path.append(os.path.abspath('.'))
Copy link
Member

Choose a reason for hiding this comment

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

There's a lot of unnecessary churn here; you really should configure black/pre-commit/whatever to only work on the diff you actually touched.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll strip out the churn.

license.rst
../citing.rst
../resources/index.rst
Copy link
Member

Choose a reason for hiding this comment

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

Should we move this directory?

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe, but that would change the URLs.

@tacaswell tacaswell force-pushed the doc_use_pydata_theme branch 2 times, most recently from ef82c2d to e4e4919 Compare June 19, 2021 18:20
@tacaswell
Copy link
Member Author

At this point, I do not think my struggles to get this working are of interest to anyone so squashed.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

I'm fine with this going in as a first version. Still needs a bit of polishing here and there, bit iIt's simpler to do additional refinement incrementally.

@tacaswell tacaswell merged commit f937134 into matplotlib:master Jun 21, 2021
@tacaswell tacaswell deleted the doc_use_pydata_theme branch June 21, 2021 15:09
@tacaswell
Copy link
Member Author

Given the two approvals I pushed the button on this!

@jklymak
Copy link
Member

jklymak commented Jul 22, 2021

We thought to try reverting this to see if the doc build would actually speed up, but Github cannot revert it "the content may have changed since it was merged." so if we try to revert, it'll be manual.

@timhoffm
Copy link
Member

Um, can‘t you simply brach from an old commit? It‘s not exactly the same repo state and docs then, but I suppose that shouldn’t make the difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to pydata-sphinx-theme
4 participants