At the sprints for PyData NYC, as I was setting up the repo for local development, I ran into an issue with installingninja directly from the dev requirements file. After chatting with@story645, she suggested I update the documentation and make a separate section to make external dependency onninja more clear.

This PR changes the docs page to add a new section for theninja external dependency, and removes it from the Python section. I also addedninja to the list of external dependencies in the documentation.

PR checklist

[N/A] "closes #0000" is in the body of the PR description tolink the related issue
[N/A] new and changed code istested
[N/A]Plotting related features are demonstrated in anexample
[N/A]New Features andAPI Changes are noted with adirective and release note
Documentation complies withgeneral anddocstring guidelines

github-actionsbot reviewed

Nov 5, 2024

View reviewed changes

Copy link

github-actionsbot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join uson gitter for real-time discussion.

For details on testing, writing docs, and our review process, please seethe developer guide

We strive to be a welcoming and open project. Please follow ourCode of Conduct.

jimmyshah changed the title~~Update page to note installation for ninja library~~DOC: Update page to note installation for ninja library

Nov 5, 2024

github-actionsbot added the Documentation: devdocsfiles in doc/devel label

Nov 5, 2024

story645 previously requested changes

Nov 6, 2024

View reviewed changes

Copy link

Member

story645 left a comment

Choose a reason for hiding this comment

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

The big thing I'm being nitpicky about is the order of bullets b/c ninja is mandatory and doc build dependencies not so much

doc/devel/development_setup.rst Outdated

		@@ -212,7 +212,7 @@ Additionally, the following non-Python dependencies must also be installed local

		* :ref:`c++ compiler<compile-dependencies>`
		* :ref:`documentation build dependencies <doc-dependencies-external>`

		* :ref:`ninja <ninja-dependencies>`

Copy link

Member

story645Nov 6, 2024

Choose a reason for hiding this comment

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

can you make this the first thing in the list since you need ninja before installing c++ -> basically mirroring the order in the dependencies file

Copy link

ContributorAuthor

jimmyshahNov 21, 2024

Choose a reason for hiding this comment

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

@story645 done!

doc/install/dependencies.rst OutdatedShow resolvedHide resolved

Copy link

Member

ksunden commentedNov 7, 2024

I think my gut reaction is that Ninja is not special enough itsown section here. I will acknowledge that it is in a bit of an odd place in that it isreally an external (non-python) dependency, but they happen to package it on pypi, which is sometimes the most convenient place for Python users to get it.

My current leaning would be to put it in the "Compilers" section (perhaps with a section rename to e.g. "Compilers and external build tools" or similar) as really that is what it is.

Copy link

Member

story645 commentedNov 7, 2024•
edited
Loading

My current leaning would be to put it in the "Compilers" section (perhaps with a section rename to e.g. "Compilers and external build tools" or similar) as really that is what it is.

@tacaswell suggested similar, only reason I suggested different section is b/c the compilers section is so rich w/ c++ compiler information. Which maybe:

Compilers and external build tools
- compilers
- ninja

But also if we're running that away with it, if@jimmyshah doesn't updated in a few days than I may just add a commit to this PR that makes those changes.

Copy link

ContributorAuthor

jimmyshah commentedNov 19, 2024

Apologies for the delay here, just catching up on this now. I'm having a little bit of trouble following the conversation:

Where is the "Compilers" section in the docs? The closest heading I could find was this:https://matplotlib.org/stable/install/dependencies.html#compiled-extensions

If@story645 wants to push this through, also totally fine by me. I can try seeing this through as well, whatever works!

Copy link

Member

story645 commentedNov 19, 2024•
edited
Loading

Where is the "Compilers" section in the docs? The closest heading I could find was this:https://matplotlib.org/stable/install/dependencies.html#compiled-extensions

It's the one you linked to, those are the tools for building compiled extensions. And would much prefer that you do it if you're up for it!