Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Add plotly[express] extra for easily installing Plotly Express dependencies #4644

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

Merged
emilykl merged 21 commits into master from better-pandas-warning
Nov 22, 2024

Conversation

@ndrezn
Copy link
Member

@ndrezn ndrezn commented Jun 19, 2024 •
edited by emilykl
Loading

Closes #2279

This PR:

  • Adds the install shortcut plotly[express], explained in more detail below
  • Renames the following files, to match conventions used in other Plotly repos:
    • requirements.txt to requires-install.txt
    • optional-requirements.txt to requires-optional.txt
  • Adds an additional file requires-express.txt, which contains the dependencies required by Plotly Express which are not required by the base Plotly package (currently, this is just numpy)
  • Adds additional detail to the error message displayed when a user tries to import Plotly Express without having numpy installed
  • Does a very light round of cleanup on requires-optional.txt, removing outdated dependencies which are definitely not needed anymore

This PR adds the ability to install Plotly with:

pip install plotly[express]

which bundles numpy in the install, which is required by plotly.express,

This also revises the error message raised when using plotly.express if numpy is not installed, providing further instructions on how to install.

Open to ideas on better wording.

(plotly.py) ➜ plotly.py git:(better-pandas-warning) python
Python 3.12.3 (main, Apr 9 2024, 08:09:14) [Clang 15.0.0 (clang-1500年3月9日.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import plotly.express
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
 File "/Users/nathandrezner/Plotly/plotly.py/packages/python/plotly/plotly/express/__init__.py", line 10, in <module>
 raise ImportError(
ImportError: Plotly Express requires numpy to be installed. You can install numpy using pip with:
$ pip install numpy
Or install Plotly Express and its dependencies directly with:
$ pip install "plotly[express]"
You can also use Plotly Graph Objects to create a large number of charts without installing
numpy. See examples here: https://plotly.com/python/graph-objects/

Copy link
Member Author

ndrezn commented Jun 19, 2024

We can plausibly drop the note about graph objects but basically I want some sort of explanation of why pandas isn't a dependency.

Copy link
Contributor

emilykl commented Jun 19, 2024

We shouldn't recommend pip install plotly.express , I believe it only exists for backwards compatibility and it's confusing to have multiple install paths.

If we want to have a one-liner "install everything" command that should be a change made inside Plotly.py.

Copy link
Member Author

ndrezn commented Jun 19, 2024

I was thinking of adding:

pip install plotly[express]

as an option which goes with usual pip patterns for optional dependencies. If we went that route I would expect that we drop support for pip install plotly.express entirely.

Copy link
Contributor

@gvwilson gvwilson left a comment

Choose a reason for hiding this comment

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

I assumed from the PR title that this was going to improve the install-time or runtime error messages for "you need Pandas" but there seem to be other changes as well? I trust you...

Copy link
Member Author

ndrezn commented Jun 19, 2024

@gvwilson there are other changes following @emilykl 's comment. It might make sense to break this into two PRs to be honest...

@ndrezn ndrezn changed the title (削除) Improve no pandas warning (削除ここまで) (追記) Add optional download requirements for dev and plotly express (追記ここまで) Jun 19, 2024
Copy link
Contributor

👍 on breaking the PR - thank you

Copy link
Contributor

emilykl commented Jun 19, 2024

IMO the naming should be plotly[pandas] rather than plotly[express] -- much more clear what it's actually doing.

I'm in favor of this change, but let's not merge it right away. This PR changes a number of lines in setup.py so it's not just an addition; it could feasibly break the install process for some users. Need to do really solid QA to make sure it runs smoothly.
ndrezn and LiamConnors reacted with thumbs up emoji

@ndrezn ndrezn force-pushed the better-pandas-warning branch from 83fb8c8 to e2bcd8b Compare June 19, 2024 21:33
Copy link
Member Author

ndrezn commented Jun 19, 2024

@gvwilson on further thought I think it makes sense to keep these changes in one PR. The text in the warning message is directly related to the installation recommendations that are also added in this PR -- I've updated the title & description to make the primary change more clear.

Copy link
Contributor

👍

ndrezn and others added 2 commits June 20, 2024 07:08
Copy link
Member Author

ndrezn commented Jul 29, 2024 •
edited
Loading

@emilykl is this PR good to go with the latest changes?

@gvwilson gvwilson removed their assignment Aug 2, 2024
@gvwilson gvwilson added feature something new P2 considered for next cycle infrastructure build process etc. labels Aug 13, 2024
Copy link
Collaborator

The benefit is we recommend a specific version number for compatibility. I don't think we have issues with different versions of Pandas but this makes it easy to pin a version of Pandas for use with Plotly if we ever encounter issues.

In fact we're doing that here, where we install the latest pandas for new versions of Python but force older pandas for older Python. People in the original issue were having funky errors caused by this incompatibility: #2279 (comment)

Oh ok that makes sense! Maybe for documenting the usage of that we could make that more clear?

contributing.md Outdated
```bash
(plotly_dev) $ pip install -r packages/python/plotly/requirements.txt
(plotly_dev) $ pip install -r packages/python/plotly/optional-requirements.txt
(plotly_dev) $ pip install -r packages/python/plotly/requires-install.txt
Copy link
Member

@LiamConnors LiamConnors Nov 20, 2024

Choose a reason for hiding this comment

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

Is this step required in contributing.md? I think it's covered by the editable install of plotly further down.

emilykl reacted with thumbs up emoji
Copy link
Contributor

Choose a reason for hiding this comment

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

@ndrezn @emilykl if you can spare time today, it would be great to get this one in - thanks

emilykl reacted with thumbs up emoji
Copy link
Contributor

Choose a reason for hiding this comment

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

good catch @LiamConnors , I'll remove this

(I verified that the setup instructions still work without this step, at least on Mac)

Copy link
Member

Looks good to me. Could you add a changelog entry for it too.
ndrezn reacted with thumbs up emoji

@emilykl emilykl changed the title (削除) Add optional download requirements for dev and plotly express (削除ここまで) (追記) Add plotly[express] extra for easily installing Plotly Express dependencies (追記ここまで) Nov 22, 2024
@emilykl emilykl merged commit c27c49d into master Nov 22, 2024
5 checks passed
@emilykl emilykl deleted the better-pandas-warning branch November 22, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexcjohnson alexcjohnson alexcjohnson left review comments

@LiamConnors LiamConnors LiamConnors approved these changes

@emilykl emilykl Awaiting requested review from emilykl

@marthacryan marthacryan Awaiting requested review from marthacryan

+1 more reviewer

@gvwilson gvwilson gvwilson approved these changes

Reviewers whose approvals may not affect merge requirements

Labels

feature something new infrastructure build process etc. P2 considered for next cycle

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Requires pandas, but not documented?

AltStyle によって変換されたページ (->オリジナル) /