X Tutup
Skip to content

setup.py: Recommend installation command for pkgs#6575

Merged
tacaswell merged 1 commit intomatplotlib:masterfrom
AbdealiLoKo:ajk/setup
Jul 12, 2016
Merged

setup.py: Recommend installation command for pkgs#6575
tacaswell merged 1 commit intomatplotlib:masterfrom
AbdealiLoKo:ajk/setup

Conversation

@AbdealiLoKo
Copy link
Copy Markdown
Contributor

If a package is not found, provide a better error message
that also explains how to install the package or gives the
user a link explaining what to do to install the package.

The supported methods are:

  • Provide a link for windows
  • Provide a command using a package manager for the OS
    (For example: apt-get for Ubuntu/Debian, dnf and yum for
    Fedora/CentOS/RHEL, brew and port for OSX)

Fixes #6546

@AbdealiLoKo
Copy link
Copy Markdown
Contributor Author

This is still a work in progress, I haven't tested it to my satisfaction - but would love some feedback on the method used as I've never contributed to matplotlib before.

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jun 12, 2016
pkg_name = self.pkg_names.get(manager, None)
if pkg_name:
try:
_ = check_output(["which", manager],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is a shutil.which, but unfortunately, it's 3.3+. It's probably not worth it to conditionally call it, but can you add a comment about switching to it when we drop 2.7?

@AbdealiLoKo
Copy link
Copy Markdown
Contributor Author

Bump - reminder for review of this PR

setupext.py Outdated
def install_help_msg(self):
"""
The help message to show if the package is not installed. The help
message shown depends on whether some class variables are present.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add a note to this docstring explaining how to add the feature to sub-classes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

If a package is not found, provide a better error message
that also explains how to install the package or gives the
user a link explaining what to do to install the package.

The supported methods are:
 - Provide a link for windows
 - Provide a command using a package manager for the OS
   (For example: apt-get for Ubuntu/Debian, dnf and yum for
    Fedora/CentOS/RHEL, brew and port for OSX)
@tacaswell
Copy link
Copy Markdown
Member

👍 I'll merge this as soon as CI passes again (just to be paranoid).

@AbdealiLoKo
Copy link
Copy Markdown
Contributor Author

@tacaswell Do you recommend we use distro instead of platform.linux_distribution as users will get a DeprecationWarning when they run this on py3.5 Similar issue: pypa/pip#3823

@tacaswell
Copy link
Copy Markdown
Member

It will only be seen if things go wrong anyway 😈 .

Given the amount of sound and fury in the upstream bug, I am inclined to let this shake it self out a bit more before jumping on a solution.

On the other hand, better to deal with it now than later (when we have forgotten).

@AbdealiLoKo
Copy link
Copy Markdown
Contributor Author

Maybe we could merge the PR but keep the issue open or create a new tracking issue to keep track of it ?

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.

4 participants

X Tutup