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: Update building docs to use Meson #24573

Merged
merged 6 commits into from
Sep 1, 2023
Merged

Conversation

melissawm
Copy link
Member

@melissawm melissawm commented Aug 29, 2023

Removes mentions of runtests.py when appropriate and replaces them with equivalent spin commands.

A few pending questions:

  • What to do with the custom blas/lapack instructions? (I know we talked about ditching those in favor of the SciPy documentation but our docs seem much more detailed and I'm hesitant to just delete.
  • I haven't updated https://numpy.org/doc/stable/dev/development_advanced_debugging.html yet, could do it on a follow-up or on a separate commit.
  • There are a couple of mentions to site.cfg that I didn't remove yet either since I'm not 100% sure of the replacement (if any).

I haven't completely redone the docs to mirror SciPy as I still feel there is justification for the current organization, but I'm happy to reconsider.

Partly addresses #23981

@melissawm melissawm added 04 - Documentation Meson Items related to the introduction of Meson as the new build system for NumPy labels Aug 29, 2023
@melissawm melissawm force-pushed the meson-docs branch 2 times, most recently from 07cf915 to 60da2d7 Compare August 29, 2023 02:04
Copy link
Member

@HaoZeke HaoZeke left a comment

Choose a reason for hiding this comment

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

Looks great, barring two small comments.

doc/source/user/building.rst Outdated Show resolved Hide resolved
doc/source/user/building.rst Show resolved Hide resolved
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks Melissa! This looks good to me to merge, after the minor open comments from Rohit and me are addressed. Larger changes can be done in follow-up PRs, this is already very helpful.

What to do with the custom blas/lapack instructions? (I know we talked about ditching those in favor of the SciPy documentation but our docs seem much more detailed and I'm hesitant to just delete.

None of that applies anymore, and there's no replacement right now beyond what's in the SciPy docs. So it should be deleted. Once there are replacement CLI flags, I'll add them to the docs in the same PR as their implementation.

I haven't updated https://numpy.org/doc/stable/dev/development_advanced_debugging.html yet, could do it on a follow-up or on a separate commit.

That's fine. That section looks a bit out of date, it'd need more work beyond updating the couple of runtests.py usages (e.g., use of sanitizers changed/improved). I think we can leave that alone here, and perhaps ask Nathan or Sebastian to update it.

There are a couple of mentions to site.cfg that I didn't remove yet either since I'm not 100% sure of the replacement (if any).

The replacement is a .pc file, as documented in http://scipy.github.io/devdocs/building/blas_lapack.html#using-pkg-config-to-detect-libraries-in-a-nonstandard-location.

I haven't completely redone the docs to mirror SciPy as I still feel there is justification for the current organization, but I'm happy to reconsider.

I think that's fine to leave for now, but my opinion is still that there's very little of value here. The build options really are pretty much identical. Organization wise, I find the NumPy docs very difficult to navigate (e.g., both the sidebar and the start of the page contents at https://numpy.org/devdocs/dev/index.html are very confusing). In addition, the NumPy docs don't help the user/contributor with system dependencies for Windows/macOS/Linux, using conda, other system dependencies, or a simple quickstart to contributing. And things like pip install build_dependencies.txt without taking into account conda users are just plain wrong and will break the contributors environment.

I'm not seeing what to retain from the current docs, but I think you have some concrete sections in mind. I suggest opening a new follow-up issue to identify those. I can't say much more without knowing what those sections are.

doc/source/dev/development_environment.rst Outdated Show resolved Hide resolved
doc/source/dev/howto_build_docs.rst Outdated Show resolved Hide resolved
doc/source/dev/howto_build_docs.rst Outdated Show resolved Hide resolved
doc/source/user/building.rst Outdated Show resolved Hide resolved
@ngoldbaum
Copy link
Member

I think we can leave that alone here, and perhaps ask Nathan or Sebastian to update it.

After this gets merged I'll try to do it as a followup.

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Aug 29, 2023
@charris charris added this to the 1.26.0 release milestone Aug 29, 2023
@melissawm
Copy link
Member Author

Here's a second take - I believe I adressed most of the comments, let me know if there's anything else that is needed.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @melissawm, the updates looked good. On a closer read I found some more issues with detailed commands, and one conceptual one (use of spin in the user guide rather than the contribution guidelines). I'll push some fixes.

*Note: for build instructions to do development work on NumPy itself, see*
:ref:`development-environment`.
To build NumPy with meson, you can use the
`spin <https://github.com/scientific-python/spin>`_ utility. First, install the
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look right. spin should only be used in the development/contribution instructions, it is not useful to users who simply want to build from source. And it doesn't have the -n/--no-build flag like runtests.py does, so it can't be used right now to run the tests on a numpy package installed in the user's environment.

benchmarks/README.rst Outdated Show resolved Hide resolved
benchmarks/README.rst Outdated Show resolved Hide resolved
@melissawm
Copy link
Member Author

Thanks for catching those!

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

All right, seems we're all good here now. Let's squash-merge so this is easy to backport. Thanks Melissa & all reviewers!

@rgommers rgommers merged commit 8381273 into numpy:main Sep 1, 2023
4 checks passed
charris pushed a commit to charris/numpy that referenced this pull request Sep 1, 2023
Removes mentions of runtests.py when appropriate and replaces them with
equivalent spin commands (or pytest in a few cases).

[skip azp]

Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Sep 1, 2023
@charris charris removed this from the 1.26.0 release milestone Sep 1, 2023
charris pushed a commit to charris/numpy that referenced this pull request Nov 11, 2023
Removes mentions of runtests.py when appropriate and replaces them with
equivalent spin commands (or pytest in a few cases).

[skip azp]

Co-authored-by: Ralf Gommers <ralf.gommers@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
04 - Documentation Meson Items related to the introduction of Meson as the new build system for NumPy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants