-
-
Notifications
You must be signed in to change notification settings - Fork 654
Improve Windows installation instructions #37184
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You said that the installation on Ubuntu is currently broken. Is this specific to WSL?
What I wrote in the PR description was based on watching a student try it on his computer 2 weeks. I can't test it myself. Just now I tested the sagemath package on the ubuntu:jammy Docker image. It installs and tests OK as of today. In any case, the Ubuntu package is stuck at version 9.5, so for this reason alone it should not be our primary recommendation. |
Thanks for the details. But this argument (especially your last point) applies to plain ubuntu as well. So I would prefer to simply change the how to install on ubuntu section to give priority to conda, and then leave the wsl section untouched. |
We do not have such a section. |
Sorry, I meant the "linux" section. |
That does not make much more sense either. The Linux section makes different recommendations based on the version of Sage that is found on the distribution. The problem that I am solving here in this PR is that there are too many indirections, each of which can confuse our potential users. That's what "streamlining" means in the PR description. |
The same would then also apply to Windows, as it is possible to use many different linux distros via wsl (as we also say in the docs). |
This is not an interesting discussion. |
Okay, let me see if I can summarize:
It will take me a little bit to test whether this actually works. But I do have access to a Windows computer if my son lets me on it 😄 However, I also don't want to get in the middle of the myriad meta-discussions going on around platforms etc. So I'm going to take the point of view that "Windows users probably need very explicit instructions, and shouldn't be asked to go to a scary Linux section" and not worry about Linux instructions, which however certainly might need updating as well. Technical question: do we expect that once |
Yes, that's also my viewpoint here. If not for anything else, then certainly for keeping the focus of the PR as narrow as possible. After all, even with this deliberately narrow focus, it has been sitting for 3 weeks.
No, |
I'm setting it to blocker because it needs to make its way in the stable release. |
I'll take a look once I get access to a Windows machine. By the way, any sense on why the Ubuntu is stuck on such an old version? (Probably this was discussed ad nauseam elsewhere, sorry.) |
I think maintainer burnout + possibly a problem with brial (hence #36380) |
Removing blocker as there are problems with other PRs, see #37428 for more details. |
How would this PR, 1 change to a documentation file, cause such a problem? |
So this is interesting - at least on the most modern Windows, you actually don't have to install Ubuntu any more either. It just automatically chooses Ubuntu for you. (See here.) It also should automatically use WSL 2. So we may want to update that part (we should still remind users of older versions about those things, of course). I'll see about installing Sage now. |
Additionally, you have to enable virtualization in BIOS. That could be annoying for some, and is definitely worth mentioning. (For instance, on my son's computer I had to just do this, otherwise I got errors when actually trying to start WSL.) |
And then create a UNIX username and then restart shell after installing mamba ... wow, there are a lot of steps. Anyway, that's mostly documented in the Windows link. But the mamba thing is a little confusing because samba is default installed already, and a user might think there was a typo in the instructions. |
It works!!! Though I can't figure out how to get plots to show up; any thoughts? I tried this link (which should totally be linked on the Windows install page ...) but it didn't seem to work. Also, I'd still recommend pointing out other Linux options somehow, and streamlining the instructions to point out that most people will not need the Windows store now. Because your mileage may vary, and we don't want monolith instructions. Maybe for older Windows versions. But on this Windows 11 system, it worked. |
Also, how fragile is this line?
|
This is actually documented here but not in the basic instructions. |
I'm putting needs work mostly because there are just enough tricky bits that simply saying "do this" isn't enough for a lot of users (notably the BIOS issue), as well as somewhat outdated (for most) like choosing the Ubuntu. Also, I'd like to know if there is a good way to avoid the specific Python version reference, or to automatically update it when necessary. But in principle the conda piece works fine, which is presumably the main point. |
@kcrisman Many thanks for testing and the detailed suggestions.
I've updated the steps and links to refer to these streamlined instructions, please take a look.
I've added this step |
I've just cut out the outdated Docker/WSL stuff from that wiki page and replaced it by a link the Sage installation guide. I don't know if it makes sense to work on the plotting support instructions that are provided there, or we should just delete them. |
@kcrisman <https://github.com/kcrisman> I believe recording a "-1" vote
is for the process proposed in
https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ, which is itself
still being voted on until Mar 13.
Thanks, I had seen that but not realized it was implemented, and I admit I
didn't read it carefully. I guess I'm +1 for this, because it's easy to
address the concerns in a new PR based on this.
Update: but it looks like maybe this is resolved in the most recent commit
and this discussion is moot?
… Message ID: ***@***.***>
|
I have merged #37588 (thanks @williamstein for testing that) and updated the instructions from there. I'm setting back to "blocker" because this really should be merged in 10.3. |
@kcrisman With these changes merged, do you want to test again, or can we merge? |
…orge <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Based on the discussion in https://groups.google.com/g/sage- devel/c/GaQHdu0Q6UU and sagemath#37184, we now follow the offical recommendation and use Miniforge over the soft- deprecated Mambaforge. Since we are exclusively testing on miniforge, this change also aligns the docs with the ci. Moreover, I've removed the recommendation to install mamba (using `conda install mamba`) which is not recommended according to the [offical docs](https://mamba.readthedocs.io/en/latest/installation/mamba- installation.html#existing-conda-install-not-recommended). ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] The title is concise and informative. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#37588 Reported by: Tobias Diez Reviewer(s): Matthias Köppe, roed314, William Stein
I'm posting to record a vote of -1 on behalf of Tobias Diez. |
Tobias added the "disputed" label and has requested that it be removed, so I am doing so. Also, based on the policy stated at https://groups.google.com/g/sage-devel/c/XDvKkMRoDk4/m/0yrtdKkGAwAJ, "if there is disagreement about whether a PR should be marked as a blocker, mark it as critical instead." So I am marking this as critical. |
How can there be a vote if the PR is not "disputed" any more? |
In my opinion, if it's not disputed, "-1" votes have no clear impact, other than to let developers know that someone has some objections. |
Ideally someone should test again, though William's other testing is good. I just worry about Windows being wonky. However, I do vote +1 for this change overall, to be clear. Comments on testing:
Addressing the issue implicitly raised, this comment suggests that, while Tobias is currently voting -1 due to duplication, it might just as easily be possible to have a quick followup PR that then uses some standard rst tool to remove that duplication. I don't really care if it happens here or there, but it does seem helpful in terms of preventing doc rot. |
Sorry for missing this. Keeping the documentation source a simple .rst file (instead of generating it somehow from some template file so that we could refer to a variable) -- this is to keep the structure of our documentation sources as simple and straightforward as possible -- this serves the contributors to the documentation. The cost of this (= the cost of updating this occurrence) is low, as the update happens only about once a year, and the places to change are easy to find, in particular for those who know how to make the updates of the supported Python versions. |
FWIW, I recommend not to take such rationalizations at face value. |
Sorry for missing this.
No problem.
The cost of this (= the cost of updating this occurrence) is low, as the
update happens only about once a year, and the places to change are easy to
find, in particular for those who know how to make the updates of the
supported Python versions.Message ID:
***@***.***>
Okay, I guess we do have to search for that every so often ...
|
Tobias is currently voting -1 due to duplication
FWIW, I recommend not to take such rationalizations at face value.
Thankfully, I have the margin to do so, though I understand why others
might not.
… Message ID: ***@***.***>
|
…y miniforge installer
Done in d3a2a16 |
…ection, point to Launching
Done in 4eacb7b, please take a look. |
Documentation preview for this PR (built with commit 4eacb7b; changes) is ready! 🎉 |
Thanks for addressing all these concerns. While I still hope for an nth iteration of a double click installer, this is fairly painless as such things go in our Windows ecosystem experience. |
@kcrisman Thanks for the review! |
@vbraun Please merge. Can't set to blocker because policy. |
We streamline the installation instructions for Windows by including conda-forge instructions.
Sage is stuck at version 9.5 in Ubuntu, the default Linux distribution on WSL, so this should not be our primary recommendation.
Preview: https://deploy-preview-37184--sagemath.netlify.app/html/en/installation/
Alternatives / follow-ups:
📝 Checklist
⌛ Dependencies