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

Don't require pyarrow on 3.9, and show a warning on custom components #2452

Merged
merged 10 commits into from
Dec 16, 2020

Conversation

akrolsmir
Copy link
Contributor

@akrolsmir akrolsmir commented Dec 11, 2020

Fixes #2356 #2001

Current Approach (12/16):

  1. In Pipfile, specify that PyArrow is only installed on Python < 3.9
  2. In all Streamlit versions, dynamically import pyarrow when Custom Components are created; show a nice error message if pyarrow is not installed
  3. Once PyArrow supports Python 3.9 (ETA 2020 Jan), revert this PR, then update docs to say Streamlit supports 3.9 officially
  4. (Future) Consider resolving watchdog error by excluding watchdog on Mac, permanently?

Tested locally on 3.9 and 3.8, verified components still work on 3.8.
Get the wheel file: https://share.streamlit.io/streamlit/core-previews/pr-2452

.
.
.
.
.

Old Proposal (12/14):

  1. Create separate wheelfiles for python 3.6-3.8 and python 3.9
  2. Exclude pyarrow dependency from python 3.9
  3. In all Streamlit versions, dynamically import pyarrow when Custom Components are created; show a nice error message if pyarrow is not installed
  4. (Future) Also fix up watchdog error with different wheels for Mac?

Notes (12/11):

  • Actually, 1. doesn't look so easy anymore. I tried editing the setup.py file thinking it would get run on each pip install, but actually it only gets run when we package the wheel file.
  • The wheel file itself does not contain setup.py, but rather has a hardcoded list of dependencies for pip to install (you can unzip the whl and look under streamlit-0.72.0.dist-info/METADATA). So the dependencies are locked in by the time we upload the wheel file; and when users pip install streamlit, there's no code that we control that decides which requirements to install.
  • By that same reasoning, it's not clear whether the watchdog fix in setup.py works for our users either???
  • Would love to know if I'm wrong on any of the above (eg if pip install does not install from an uploaded wheel, but rather a source folder including setup.py?)

Notes (12/10):
From @tvst:
A temporary patch for the PyArrow thing could look like this:

  1. When you pip-install Steramlit, we can check the Python version (during the installation process). If Python 3.9, don't include Arrow.
  2. Then when you try to use Components, put a try/catch around the import Arrow statement. If it fails, show a message with a super nice user-readable explanation of whats up and how to solve it.
  • This PR is still completely untested; waiting for PR Preview to make a wheelfile. Wheelfile here
  • Could use some Product workshopping on the specific error text we show
  • Biggest worry is this comment:

Using Python 3.8 does not solve the problem. Seems to me, pyarrow does not provide a wheel for Mac OS 11 yet.

If true, we'd want to change up our setup.py to try and detect Mac OS 11???

@akrolsmir akrolsmir requested a review from a team December 11, 2020 01:45
@karriebear karriebear self-assigned this Dec 11, 2020
@akrolsmir
Copy link
Contributor Author

Preview:

Screen Shot 2020-12-11 at 10 28 51 PM

@akrolsmir akrolsmir changed the title Don't require pyarrow on 3.9, and show a warning on custom components Build wheels for Python 3.9 without pyarrow Dec 12, 2020
@polm
Copy link

polm commented Dec 12, 2020

Regarding this comment:

Using Python 3.8 does not solve the problem. Seems to me, pyarrow does not provide a wheel for Mac OS 11 yet.

This bit me too with some packages I maintain. The new Mac architecture can use packages for the old Mac architecture, but pip needed some changes to realize that, so it only works with pip 20.3 or higher. See this issue for more details.

Pip 20.3 was released Dec 1, so after the comment you quoted. So if someone on 3.8 or lower on the new Mac has issues you can probably just tell them to upgrade pip.

@karriebear
Copy link
Contributor

Curious did you give the extras_require a try? It looks like we could add a dependency based on python version but haven't tried yet. https://discuss.python.org/t/adding-a-default-extra-require-environment/4898/7. If this works, I would personally prefer this approach.

My concerns with the current approach is mainly regarding the release and distribution process. Concerns are easily testable but would want to confirm if we have a 3.9 version on pypi, would pip install be smart enough to get the right wheel for each environment. For releases, should we test both wheels? May have to modify our release scripts to ensure the right wheel is installed for each testing, etc.

@akrolsmir
Copy link
Contributor Author

akrolsmir commented Dec 14, 2020

Curious did you give the extras_require a try? It looks like we could add a dependency based on python version but haven't tried yet. https://discuss.python.org/t/adding-a-default-extra-require-environment/4898/7. If this works, I would personally prefer this approach.

I have not -- the problem with extras_require afaict is that it's part of setuptools, meaning that it would only work for the source distribution, aka when users install from our .tar.gz using pip install streamlit --no-binary. Our wheel (aka bdist, binary distribution) would presumably strip out the extras_require as it takes out all of the setuptools code, compiling the requirements into the METADATA file.

I guess we could change our guides to recommend pip install streamlit --no-binary, or alternatively stop distributing wheel files altogether so pip install streamlit would always pick the .tar.gz? (Until pyarrow starts supporting 3.9). One potential problem: do our users need to install any additional dependencies to compile the .tar.gz into a working version of Streamlit?

My concerns with the current approach is mainly regarding the release and distribution process. Concerns are easily testable but would want to confirm if we have a 3.9 version on pypi, would pip install be smart enough to get the right wheel for each environment. For releases, should we test both wheels? May have to modify our release scripts to ensure the right wheel is installed for each testing, etc.

Yeah, this is my primary concern too. I don't think we want to maintain multiple wheels long-term, this is really just a workaround for the pyarrow 3.9 thing, and we should take it out as soon as pyarrow provides wheels for 3.9. Until then, though, I think the added maintenance burden would be worth making the pip install streamlit experience suck less for our users.

@karriebear
Copy link
Contributor

I have not -- the problem with extras_require afaict is that it's part of setuptools, meaning that it would only work for the source distribution, aka when users install from our .tar.gz using pip install streamlit --no-binary. Our wheel (aka bdist, binary distribution) would presumably strip out the extras_require as it takes out all of the setuptools code, compiling the requirements into the METADATA file.

From my understanding, the setuptools code is what creates our wheel so if you pass extras_require it should put the conditional dependency into the METADATA file. Based on the example in the link we should get a line like below added which should hopefully only install numpy if python version > 3.4.
Requires-Dist: numpy ; python_version>"3.4"

I think the issue with your first attempt was that you were evaluating the conditional requirements on compile time whereas adding the conditional requirement as a parameter to the setuptools command will pass the conditional requirement to evaluate on install time.

@karriebear
Copy link
Contributor

Did a very not thorough test for extras_require and it looks to do what we want. Draft PR for it #2464

@akrolsmir
Copy link
Contributor Author

Note for posterity: Karrie was absolutely right and I was wrong above; extra_requires would have worked perfectly by updating the METADATA file to conditionally install pyarrow. I got a bit confused about the difference between install_requires and extra_requires, the former might work too. But the easiest solution of all turned out to be to not muck with Setuptools; Pipenv already provides a way of specifying requirements conditional on Python versions, so let's just use that.

@akrolsmir akrolsmir changed the title Build wheels for Python 3.9 without pyarrow Don't require pyarrow on 3.9, and show a warning on custom components Dec 16, 2020

And if you're using Streamlit Sharing, add "pyarrow" to your requirements.txt."""
)

Copy link
Contributor

Choose a reason for hiding this comment

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

probably not worth it but just noting this down, it looks all custom components will not work regardless or not if the component uses tables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. The alternate approach would be to try-except specifically the areas that call on arrow_table, or to move this try-catch into arrow_table.py altogether.

In my head, cutting the boundary at "Custom Components" rather than "Custom Components using tables" was easier for users to reason about, and I imagine those who run into this issue (those inclined to install components) are technical enough not to have a huge problem with this, plus AFAICT components are not that widely, so I didn't feel like optimizing this further.

Incidentally, would be nice to have easy metrics on "What % of Streamlit Apps use Components" and "What % of Streamlit Apps are on Python 3.9".

lib/streamlit/components/v1/components.py Show resolved Hide resolved
PyArrow. Unfortunately, PyArrow does not yet support Python 3.9.

You can either switch to Python 3.8 with an environment manager like PyEnv, or
[install Streamlit with conda](https://discuss.streamlit.io/t/note-installation-issues-with-python-3-9-and-streamlit/6946/7):
Copy link

Choose a reason for hiding this comment

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

@akrolsmir : I'm a little confused by reading this line. Are we telling the user to

  1. install Python 3.8 (and streamlit) using conda as en environment manager
    OR
  2. are we saying that by using conda the user can use Python 3.9 and PyArrow

Can we make the distinction clear in the comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

install Python 3.8 (and streamlit) using any environment manager or use conda

Copy link

Choose a reason for hiding this comment

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

Cool, LGTM 👍

@asaini
Copy link

asaini commented Dec 16, 2020

LGTM 👍

@karriebear karriebear merged commit 451b7cc into streamlit:develop Dec 16, 2020
tconkling added a commit to tconkling/streamlit that referenced this pull request Dec 21, 2020
* develop:
  Extract st.container, columns, container out of delta_generator.py (streamlit#2487)
  Remove the unused variables inside protos (streamlit#2486)
  Stop running cypress-flaky-approval for each PR (streamlit#2490)
  Dynamically import git, and fail gracefully if missing (streamlit#2482)
  Bump React to 17.0.1 (streamlit#2453)
  Fix emojis (streamlit#2480)
  Add missing copyright headers (streamlit#2478)
  Stop requiring watchdog when installing Streamlit on Macs (streamlit#2470)
  Minor tweak to PyArrow warning message (streamlit#2472)
  Use a set literal (streamlit#2476)
  Add type annotations to DeltaGenerator mixins (streamlit#2475)
  Update change log
  Up version to 0.73.0
  Don't require pyarrow on 3.9, and show a warning on custom components (streamlit#2452)
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.

Installation issues with Python 3.9 on mac Big Sur
4 participants