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

Update dependencies for python3.10 #1999

Closed
wants to merge 5 commits into from

Conversation

rohithkrn
Copy link
Collaborator

@rohithkrn rohithkrn commented Nov 23, 2022

Description

Update dependencies for python3.10 support.

Had to install rust in dependencies since tokenizers pip wheel would fail without it.

copying py_src/tokenizers/processors/init.pyi -> build/lib.macosx-12-arm64-cpython-39/tokenizers/processors
copying py_src/tokenizers/trainers/init.pyi -> build/lib.macosx-12-arm64-cpython-39/tokenizers/trainers
copying py_src/tokenizers/tools/visualizer-styles.css -> build/lib.macosx-12-arm64-cpython-39/tokenizers/tools
running build_ext
running build_rust
error: can't find Rust compiler

Issue: #1959

@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #1999 (53bf3a8) into master (6fcf86d) will not change coverage.
The diff coverage is n/a.

❗ Current head 53bf3a8 differs from pull request most recent head e12076e. Consider uploading reports for the commit e12076e to get more accurate results

@@           Coverage Diff           @@
##           master    #1999   +/-   ##
=======================================
  Coverage   53.31%   53.31%           
=======================================
  Files          70       70           
  Lines        3157     3157           
  Branches       56       56           
=======================================
  Hits         1683     1683           
  Misses       1474     1474           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@lxning lxning left a comment

Choose a reason for hiding this comment

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

could you please add test log?

@lxning
Copy link
Collaborator

lxning commented Dec 9, 2022

I tried this PR on ubuntu, it does not work.

git diff ts_scripts/install_dependencies.py
diff --git a/ts_scripts/install_dependencies.py b/ts_scripts/install_dependencies.py
index 1b0484c4..2fdc6caa 100644
--- a/ts_scripts/install_dependencies.py
+++ b/ts_scripts/install_dependencies.py
@@ -67,6 +67,10 @@ class Common:
     def install_wget(self):
         pass

+    def install_rust(self):
+        os.system(
+            "curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y"
+        )

 class Linux(Common):
     def __init__(self):
@@ -148,6 +152,7 @@ def install_dependencies(cuda_version=None):
         system.install_wget()
         system.install_nodejs()
         system.install_node_packages()
+        system.install_rust()

     if platform.system() == "Linux" and args.environment == "dev":
         system.install_libgit2()

python ts_scripts/install_dependencies.py --environment dev
...
If you are using an outdated pip version, it is possible a prebuilt wheel is available for this package but pip is not able to install from it. Installing from the wheel would avoid the need for a Rust compiler.

  To update pip, run:

      pip install --upgrade pip

  and then retry package installation.

  If you did intend to build this package from source, try installing a Rust compiler from your system package manager and ensure it is on the PATH during installation. Alternatively, rustup (available at https://rustup.rs) is the recommended way to download and update the Rust compiler toolchain.
  [end of output]

note: This error originates from a subprocess, and is likely not a problem with pip.
ERROR: Failed building wheel for tokenizers

@agunapal
Copy link
Collaborator

agunapal commented Dec 9, 2022

I was looking into this. transformers version 4.11.0 doesn't work with Python 3.10, Do we have a dependency for using the version 4.11.0 of transformers @HamidShojanazeri @msaroufim

@msaroufim
Copy link
Member

I suspect the rust issue would go away if we just upgrade pip as part of CI. Regardless no I don't think we should pin to older versions of transformer, the example might break though if are using a preuploaded mar file instead of generating it for tests

@agunapal
Copy link
Collaborator

agunapal commented Dec 9, 2022

I tried upgrading pip. That didnt work.

@lxning
Copy link
Collaborator

lxning commented Dec 10, 2022

Py310 is fixed in #2031. so close this one.

@lxning lxning closed this Dec 10, 2022
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