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

tests(python): Make torch install CI-only by default #16058

Merged
merged 2 commits into from
May 6, 2024

Conversation

alexander-beedie
Copy link
Collaborator

@alexander-beedie alexander-beedie commented May 5, 2024

Streamlining the default dev environment...

  • Moves torch package from "requirements-dev.txt" to new "requirements-ci.txt".
  • Ensures that typing lint passes for polars.ml.torch with/without local torch install.
  • Renames the third_party_integration marker to ci_only to better describe its purpose.
  • Adds a make requirements-all command that installs the usual dev packages and the "CI only" packages.
  • Makes the top-level torch import in the "test_to_torch" unit tests lazy-load so that optional absence of the package doesn't cause pytest to complain when run locally (the CI runner will always have it installed, so no issue there).

We might want to make a couple of other packages CI-only by default, but for now this only covers the new torch integrations.

@github-actions github-actions bot added the internal An internal refactor or improvement label May 5, 2024
@alexander-beedie alexander-beedie changed the title tests(python): Make torch tests CI-only, defaulting to optional install of such packages tests(python): Make torch install CI-only, defaulting to optional install of such packages May 5, 2024
@alexander-beedie alexander-beedie changed the title tests(python): Make torch install CI-only, defaulting to optional install of such packages tests(python): Make torch install CI-only by default May 5, 2024
Copy link

codecov bot commented May 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 80.93%. Comparing base (eb7f939) to head (67deca2).
Report is 3 commits behind head on main.

Files Patch % Lines
py-polars/polars/ml/torch.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #16058      +/-   ##
==========================================
- Coverage   80.97%   80.93%   -0.05%     
==========================================
  Files        1386     1386              
  Lines      178382   178382              
  Branches     3059     3059              
==========================================
- Hits       144446   144372      -74     
- Misses      33448    33522      +74     
  Partials      488      488              
Flag Coverage Δ
python 74.49% <0.00%> (-0.03%) ⬇️
rust 78.15% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alexander-beedie alexander-beedie force-pushed the lazy-torch-test-import branch 2 times, most recently from b106590 to 521ab69 Compare May 5, 2024 18:20
@alexander-beedie alexander-beedie added ci Related to the continuous integration setup test Related to the test suite labels May 5, 2024
@alexander-beedie alexander-beedie force-pushed the lazy-torch-test-import branch from 521ab69 to 23efd34 Compare May 5, 2024 18:23
@alexander-beedie alexander-beedie force-pushed the lazy-torch-test-import branch from 23efd34 to e42e9f9 Compare May 5, 2024 18:32
Copy link
Member

@ritchie46 ritchie46 left a comment

Choose a reason for hiding this comment

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

Thanks Alex!

@ritchie46 ritchie46 merged commit ae66acd into pola-rs:main May 6, 2024
13 checks passed
@alexander-beedie alexander-beedie deleted the lazy-torch-test-import branch May 6, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Related to the continuous integration setup internal An internal refactor or improvement test Related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants