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

Provide stark warning against the usage of sudo pip [...] #4305

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

hmaarrfk
Copy link
Member

Description

Many users are tempted to type sudo pip

Closes #4304

We really need to update the install instructions, they are rather confusing...

Checklist

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.
  • Consider backporting the PR with @meeseeksdev backport to v0.14.x

INSTALL.rst Outdated

Will perform a valid install of scikit-image on Ubuntu 19.10, but will install
version ``0.14.2`` (even for Python 3!!) while the latest version of
scikit-image at the time of writing is ``0.16.2``. Again, **never** type::
Copy link
Member

Choose a reason for hiding this comment

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

I suggest removing specific version mentions, instead something like: "will perform a valid install of scikit-image on the system Python, but it will usually be a few major versions behind the latest release on PyPI."

Also, it's best not to sound very bossy, and instead make it strong recommendations:

"Again, we strongly recommend avoiding "sudo pip" or "sudo python -m pip""

INSTALL.rst Outdated
pip install scikit-image
pip install --user scikit-image

Again, do **not** type ``sudo pip ...``.
Copy link
Member

Choose a reason for hiding this comment

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

"We strongly advise not to use "sudo pip" to install Python packages."

INSTALL.rst Outdated
to boot, and the easiest solution might be to perform a clean install.

If you must use the system environment, which we do not recommend, read on
below.
Copy link
Member

Choose a reason for hiding this comment

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

We could link here to an external tutorial on Python environments. This just came across the software carpentry mailing list:

https://kaust-vislab.github.io/introduction-to-conda-for-data-scientists/

Copy link
Member Author

Choose a reason for hiding this comment

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

don't most core devs prefer pip?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps. If you see a cool virtualenv tutorial, let me know. ;) Last I checked, venv was not so great for maintaning multiple Python versions, but things might have evolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would put this in the sphinx warning box so it is red.

Copy link
Member

@jni jni left a comment

Choose a reason for hiding this comment

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

@hmaarrfk I left a few minor suggestions. However, this is an improvement as is, so I'll approve. Let's let other people weigh in on the wording, but I implore people not to get too bikesheddy about the language. Let's make sure people reading our instructions don't make this mistake again!

INSTALL.rst Outdated
@@ -23,7 +41,9 @@ including `Anaconda <https://www.anaconda.com/download/>`_,

On all major operating systems, install it via shell/command prompt::

pip install scikit-image
pip install --user scikit-image
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'm not sure if this is correct for venv and conda envs

Copy link
Member

Choose a reason for hiding this comment

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

You're right, I don't think this will do the right thing in a conda-env. I'm not sure about venv.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this is not the correct way to do it, this will install it in a systemwide user directory, better than encouraging sudo pip but not better than teaching about venv

$ ls /home/mark/.local/lib/python3.7/site-packages
cycler-0.10.0.dist-info                     numpy-1.17.4.dist-info
cycler.py                                   PIL
dateutil                                    Pillow-6.2.1.dist-info
decorator-4.4.1.dist-info                   __pycache__
decorator.py                                pylab.py
imageio                                     pyparsing-2.4.5.dist-info
imageio-2.6.1.dist-info                     pyparsing.py
kiwisolver-1.1.0.dist-info                  python_dateutil-2.8.1.dist-info
kiwisolver.cpython-37m-x86_64-linux-gnu.so  PyWavelets-1.1.1.dist-info
matplotlib                                  pywt
matplotlib-3.1.1.dist-info                  scikit_image-0.16.2.dist-info
matplotlib-3.1.1-py3.7-nspkg.pth            scipy
mpl_toolkits                                scipy-1.3.2.dist-info
networkx                                    six-1.13.0.dist-info
networkx-2.4.dist-info                      six.py
numpy                                       skimage

Copy link
Member Author

Choose a reason for hiding this comment

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

this was from within a conda env.

Copy link
Member

Choose a reason for hiding this comment

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

I guess this advice needs to be tested reasonably thoroughly:

  • what happens when pip is system pip
  • what happens when a venv is activated
  • what happens when a conda-env is activated?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, at this stage we need to rework the whole install procedure... working on that.

Copy link
Member

Choose a reason for hiding this comment

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

what happens when pip is system pip

On my system (Manjaro), pip install will fail without using either sudo or --user with a "permission denied" error. Likely the same for other Linux-based OSs?

what happens when a conda-env is activated

As far as I know, conda installs pip automatically in any environment with Python. Activating that environment will also point pip at the environment version. Using pip from within a conda environment works good for pure-Python packages (and dependencies) but I've frequently encountered compatibility issues when mixing binary packages from PyPI and conda's repos (doesn't help that conda ships a very old version of ld).

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 know, this whole thing of installing software is a mess. The goal of this PR is mostly to avoid people going:

  1. I tried pip install scikit-image it said permission error.
  2. I gave it permissions sudo !! because my friend told me that sudo !! is a cool shotcut.
  3. Now my whole system is broken.

Even Ubuntu 18.04 ships with a version of numpy much too old for scikit-image 0.16

https://packages.ubuntu.com/bionic/python/python3-numpy

INSTALL.rst Outdated Show resolved Hide resolved
INSTALL.rst Outdated Show resolved Hide resolved
INSTALL.rst Outdated Show resolved Hide resolved
INSTALL.rst Outdated Show resolved Hide resolved
INSTALL.rst Outdated Show resolved Hide resolved
INSTALL.rst Outdated

3. Linux operating system installation
--------------------------------------
We do not recommend using the opearting system's environment of python for a few
Copy link
Member

Choose a reason for hiding this comment

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

We do not recommend using the operating system's environment of python for a few

INSTALL.rst Outdated Show resolved Hide resolved
@alexdesiqueira
Copy link
Member

I am humbly against these kinds of "warnings". It would be better for us to present how to install skimage, not a how not to install it... IMHO we should assume that our installing instructions suffice, and trust the good judgment of our users.

@jni
Copy link
Member

jni commented Dec 3, 2019

@alexdesiqueira I don't know, I totally get what you're saying, it somewhat clutters the narrative, but at the same time, this was created because of a real user issue, and I agree with @hmaarrfk that the internet does not provide sufficient warning about this... The reality is that right now Python packaging is quite a mess and there are lots of ways to mess up an install. I do it all the time, in fact! It's only because I have experience with environments that I survive.

An up-to-date packaging, environment, and installation guide is what's really needed, but I think that would be a big book and it would be very challenging to keep up to date, because these things are changing every day.

@hmaarrfk
Copy link
Member Author

hmaarrfk commented Dec 3, 2019

@alexdesiqueira I did make an other PR that suggested rewording the current install instructions:
https://github.com/scikit-image/scikit-image/pull/4306/files

I don't know what else to do, because when I see instructions like this:
https://www.raspberrypi.org/documentation/linux/software/python.md
I know that it isn't the user's fault when they brick their computers.

@JDWarner
Copy link
Contributor

JDWarner commented Dec 3, 2019

I think it's appropriate to strongly warn against sudo pip. I've seen this create multiple issues over time, which can be challenging to debug. At this point I think it's essentially never correct to call pip with elevated user privileges.

@alexdesiqueira
Copy link
Member

alexdesiqueira commented Dec 3, 2019

I don't know what else to do, because when I see instructions like this:
https://www.raspberrypi.org/documentation/linux/software/python.md
I know that it isn't the user's fault when they brick their computers.

@hmaarrfk in this case it'd be better for us to open an issue on the RasPi documentation and point the issue 😅

The reality is that right now Python packaging is quite a mess and there are lots of ways to mess up an install.

@jni infidel! 😆 Joking aside, I understand your points and I am always on the side of the rookie user. However, @jni puts that "Python packaging is quite a mess," and while I don't fully agree with that, he has a point. The Python packaging(s) system is the strong point to keep things as simple as possible; if we set too many rules, we can create more doubt to the user.
In our page, I think we should recommend installation instructions for pip and conda and say we can help with that issues. If someone has an issue, we could say "hi, that's sad, did you try this?" and point the installation page. But again, my two BRL cents only.

@stefanv
Copy link
Member

stefanv commented Dec 6, 2019

I understand the motivation behind this PR, but I think it is a bit over the top and alarmist. Installing scikit-image via sudo pip is unlikely to destroy your operating system.

My recommendation is, under the standard pip instructions, mention that sudo pip should not be used. Also mention that, if you are using pip, it should be done in a virtual environment. Otherwise, users should use their operating system's package manager to install it (e.g., packages are available for Debian, ... not sure which others).

This should, maybe, add three sentences to the docs.

We worked really hard to simplify these instructions after they were bloated by numerous well-intended PRs. The problem is that it obfuscates the overarching narrative, and is only applicable to a small number of users.

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.

Requesting changes as per comment.

@hmaarrfk
Copy link
Member Author

hmaarrfk commented Dec 7, 2019

I'm happy to remove this "alarmist" PR.

I really don't think people should be typing "sudo anything to do with scikit-image"

The instructions that simply teach users how to install scikit-image in 2019 are proposed in
#4306

@stefanv
Copy link
Member

stefanv commented Dec 7, 2019

I agree with you, Mark. I just think we should tone it back a bit and reduce the amount of text needed.

Base automatically changed from master to main February 18, 2021 18:23
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.

ImportError: No module named skimage.io
8 participants