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

Update installation page #595

Merged
merged 26 commits into from
Dec 23, 2024
Merged

Update installation page #595

merged 26 commits into from
Dec 23, 2024

Conversation

lucascolley
Copy link
Member

@lucascolley lucascolley commented Dec 16, 2024

  • Modernise discussion around installing from PyPI or conda-forge
  • Include uv and pixi as options
  • Add explanation of how to install with static type stubs
  • remove mention of Python distributions and Anaconda defaults (Update installation page #595 (comment))

cc @rgommers @stefanv @jarrodmillman @tupui @jorenham

Copy link

netlify bot commented Dec 16, 2024

Deploy Preview for scipy-org ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 73a5f49
🔍 Latest deploy log https://app.netlify.com/sites/scipy-org/deploys/6769eb7c8dda3800083cb5b2
😎 Deploy Preview https://deploy-preview-595--scipy-org.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 98
Accessibility: 100
Best Practices: 100
SEO: 100
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
@jorenham
Copy link

Good stuff! Maybe it could be a good idea to mention the scipy-stubs repo, so that stuff like bug reports ends up at the right place?

content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
@lucascolley
Copy link
Member Author

Maybe it could be a good idea to mention the scipy-stubs repo, so that stuff like bug reports ends up at the right place?

I think that would be okay, although maybe sufficient to add a notice to the SciPy bug report template.

Copy link
Member Author

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

Thanks all for the comments! I have addressed all feedback as follows:

  • Removed section on Python Distributions. Talk only about conda-forge, not Anaconda defaults.
  • Discerned primarily between environment-based and project-based workflows, rather than PyPI vs conda-forge. Marked environment-based as "the traditional workflow", project-based as "recommended for new users", and "system package managers" as "simplest" (but not recommended).
  • Removed existing discussion of pip vs conda vs system vs source
  • Linked to the external docs for all tools

Let me know what you think!

content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
@lucascolley
Copy link
Member Author

shall we give a week or so for any more comments here? This is ready from my side.

Copy link
Member

@stefanv stefanv 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 @lucascolley!

Overarching comment: think of the target audience—who will be reading this guide, and why? Further keep things as simple and straightforward as possible, reducing choices only to those absolutely necessary.

content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
Copy link
Member Author

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

thanks for the review Stefan!

content/en/install.md Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
content/en/install.md Outdated Show resolved Hide resolved
Copy link
Member Author

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

I've rewritten the page, focusing on a beginner perspective. How does that look @stefanv ? And does the uv section look good to you @zanieb ?

Comment on lines +48 to +50
{{< admonition hint >}}
The second command changes directory into the directory of your project.
{{< /admonition >}}
Copy link
Member Author

Choose a reason for hiding this comment

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

@jarrodmillman, I believe the linter is suggesting changes which would break the formatting.

Copy link
Member

Choose a reason for hiding this comment

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

You could try something like

<!-- prettier-ignore-start -->
...
<!-- prettier-ignore-end -->

@matthew-brett
Copy link
Contributor

I get the feeling that it is going to be difficult and time-consuming to reach agreement on all of those open questions.

With some effort and goodwill - maybe a week to get something that has been adequately discussed and agreed? Although, as I said before, that might be harder here than elsewhere. Still, if we do move it elsewhere, and then come back with the result, is a week an acceptable timeframe?

Could we perhaps instead focus on agreeing on some sort of improvement which we can merge, then continue further discussion elsewhere? The current instructions really are quite outdated.

Honestly, if we need something now now, then I can live with the Anaconda option - but I do think it needs at least a mention and a link to the terms of service. It seems to me it would be very easy for someone in a medium-sized company to install Anaconda without realizing they could become liable to the download fees, and it would be irresponsible of us to make that easier, given the ease with which we can help people avoid it.

@lucascolley
Copy link
Member Author

With some effort and goodwill - maybe a week to get something that has been adequately discussed and agreed? Although, as I said before, that might be harder here than elsewhere. Still, if we do move it elsewhere, and then come back with the result, is a week an acceptable timeframe?

Would you be willing to open a PR yourself at that stage? I would prefer to limit the scope of my PR to the main changes I feel are important (mentioning static type stubs, mentioning venvs for pip, mentioning just Miniforge for conda installs, mentioning project-based workflows for users who are not absolute beginners, mentioning Anaconda's blog post about TOS if we keep Anaconda).

Honestly, if we need something now now, then I can live with the Anaconda option - but I do think it needs at least a mention and a link to the terms of service. It seems to me it would be very easy for someone in a medium-sized company to install Anaconda without realizing they could become liable to the download fees, and it would be irresponsible of us to make that easier, given the ease with which we can help people avoid it.

Yes, that was what I originally had in this PR.

@matthew-brett
Copy link
Contributor

Would you be willing to open a PR yourself at that stage? I would prefer to limit the scope of my PR to the main changes I feel are important (mentioning static type stubs, mentioning venvs for pip, mentioning just Miniforge for conda installs, mentioning project-based workflows for users who are not absolute beginners, mentioning Anaconda's blog post about TOS if we keep Anaconda).

That sounds reasonable to me.

Yes, that was what I originally had in this PR.

Would you consider restoring that? I think I must have missed the discussion.

Copy link
Member Author

@lucascolley lucascolley left a comment

Choose a reason for hiding this comment

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

The first thing you see now on the install page is the following tip:

image

If you click through to the beginner guide, you get three options. Firstly, a link out to https://jupyter.org/try-jupyter/lab/. Secondly, a slightly more concise version of the existing "Scientific Python Distributions" section, with links to Anaconda, WinPython and Pyzo. This includes the note:

Users in large, non-university institutions may want to read Anaconda’s helpful blog on “when is Anaconda free to use?”

Thirdly, python -m pip install scipy. However, this is accompanied with the warning:

You may see this recommended in tutorials or classes, but it is not recommended to install packages globally on your system. The recommended way to install SciPy with pip is to use a virtual environment - see Installing with pip.

and a link to the PyPA discussion of venvs for the interested reader.


For users who remain on the main installation guide, they are recommended the project-based workflows, with full instructions to get set up from scratch, and also given the option of environment-based workflows. The existing sections on system package managers and building from source are reproduced below. Then we get the section on installing with type stubs.


Please let me know if anyone sees any more blockers. I have also addressed @stefanv's concern regarding "what does a uv user need to do if they reboot their machine" in the uv section.

Copy link
Contributor

@matthew-brett matthew-brett left a comment

Choose a reason for hiding this comment

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

Some quick thoughts.

[scipy-user-guide]: https://docs.scipy.org/doc/scipy/tutorial/

<a name="distributions"></a>

Copy link
Contributor

Choose a reason for hiding this comment

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

Do they need any warnings here about losing their work if they clear their cache etc?

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure - suggestions welcome!


- [WinPython](https://winpython.github.io): Another free distribution
including scientific packages and the Spyder IDE; Windows only, but
more actively maintained and supports the latest Python 3 versions.
Copy link
Contributor

Choose a reason for hiding this comment

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

More actively maintained than Anaconda?

Copy link
Member Author

Choose a reason for hiding this comment

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

it looks like that was originally in relation to Python(x, y) - see #259. I guess we just remove that part now.

Copy link
Member Author

Choose a reason for hiding this comment

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

@CAM-Gerlach thought I'd ping in case you have any opinions on the Python Distributions section here! Hasn't been updated in a while...

content/en/beginner-install.md Outdated Show resolved Hide resolved
content/en/beginner-install.md Outdated Show resolved Hide resolved
Copy link
Member

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

Thanks, @lucascolley, for addressing the concerns around a beginner workflow. With those changes, including Matthew's phrasing around Anaconda licensing, I'm +1 on merging.


<a name="system-package-managers"></a>

## Installing system-wide via a system package manager
Copy link
Member

Choose a reason for hiding this comment

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

Since these methods are so problematic, we could consider only keeping the first sentence (warning) and not giving individual instructions for Debian, Fedora, and macos here.

Optionally, we could then show the route I mentioned elsewhere, which works perfectly well: install Python via system and then just create a virtual env using python -m venv. Since this page is no longer the beginner guide, I think that should be acceptable?

The suggestion is not needed for this PR, but if it seems like a sensible suggestion I'm happy to make a follow-up PR in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since these methods are so problematic, we could consider only keeping the first sentence (warning) and not giving individual instructions for Debian, Fedora, and macos here.

I would be happy with that, let's see what others think.

Optionally, we could then show the route I mentioned elsewhere, which works perfectly well: install Python via system and then just create a virtual env using python -m venv. Since this page is no longer the beginner guide, I think that should be acceptable?

How does that differ from https://deploy-preview-595--scipy-org.netlify.app/install/#installing-with-pip? Just installing Python with a system package manager rather than python.org? I said in another thread that I would be happy to remove the hyperlink to python.org, but I thought that listing out options for installing Python might be overkill.

The suggestion is not needed for this PR, but if it seems like a sensible suggestion I'm happy to make a follow-up PR in the future.

👍

[this guide in the SciPy docs](https://scipy.github.io/devdocs/building/index.html).
[the building from source guide in the SciPy docs][building-docs].

[building-docs]: https://scipy.github.io/devdocs/building/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.

Given the default recommendation to use uv for projects, I was wondering whether, in the future, we should adjust the above page accordingly?

I presume it can be used the same way pip is for SciPy development, adding a third code tab with uv run python dev.py and uv pip for installing dependencies?

Copy link
Member Author

@lucascolley lucascolley Dec 21, 2024

Choose a reason for hiding this comment

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

Given the default recommendation to use uv for projects, I was wondering whether, in the future, we should adjust the above page accordingly?

Yep! Although for SciPy, given the compiler dependencies, Pixi will probably be our best bet. I already use https://github.com/rgommers/pixi-dev-scipystack for all of my SciPy development. Windows support for PyTorch and JAX on conda-forge is still maturing (though actively - conda-forge/pytorch-cpu-feedstock#231), but we may be able to use PyPI on Windows to fill some gaps if anyone needs it.

A third tab for uv is probably not so useful, since it's relatively obvious what to do by following along with the pip instructions, and you have to do the same setup of system deps.

I presume it can be used the same way pip is for SciPy development, adding a third code tab with uv run python dev.py and uv pip for installing dependencies?

All that to say that we could do something like this. Or we could make a new page instead of adding a new tab, since there is much less to explain when most of the work is already done in writing the Pixi manifest. We may want to wait until somebody wants to try to merge Pixi support into the main repo, though.


Also I plan to add a note about scipy-stubs to the building from source page after this is merged.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, Lucas!

@lucascolley
Copy link
Member Author

@jarrodmillman just pinging again to ask about the lint errors - do you know if there is a way to format indented admonitions in the theme which appeases the linter?

@lucascolley
Copy link
Member Author

any further comments, on the unresolved comments or otherwise? If not, let's merge this in a few days

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.

This seems pretty good to merge, thanks Lucas and all.

content/en/beginner-install.md Outdated Show resolved Hide resolved
@stefanv
Copy link
Member

stefanv commented Dec 23, 2024

@lucascolley the paragraph following is also impacted by that last change

@lucascolley
Copy link
Member Author

thanks Stefan, addressed

@stefanv stefanv merged commit 4c947a9 into scipy:main Dec 23, 2024
5 checks passed
@stefanv
Copy link
Member

stefanv commented Dec 23, 2024

Thanks, Lucas! Sorry this got a bit more involved than anticipated ☺️

@lucascolley
Copy link
Member Author

Sorry this got a bit more involved than anticipated ☺️

I wouldn't want it to be boring :) thank you to all reviewers who gave their time here!

@lucascolley lucascolley deleted the installation branch December 23, 2024 23:42
@jorenham
Copy link

Thanks Lucas!

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.