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

feat: allow for tests to be run without Polars installed, unify exceptions more #1896

Open
wants to merge 2 commits into
base: importorskip-polars
Choose a base branch
from

Conversation

mgorny
Copy link

@mgorny mgorny commented Jan 30, 2025

CC @MarcoGorelli

Sending early to get feedback on the approaches used.

I went for:

  1. pytest.importorskip() if the whole test (or module) seemed to rely on polars.
  2. Explicit try/except ImportError: for tests that partially rely on polars — figured it would be weird to have them reported as "skipped" when the other part passed.
  3. An explicit tuple variable for expected exceptions for pytest.raises(), with appending the polars exception when polars constructor is used.

Issue #1726

mgorny and others added 2 commits January 30, 2025 12:39
Sending early to get feedback on the approaches used.
@MarcoGorelli
Copy link
Member

thanks @mgorny ! your approach looks good, will take a closer look at the weekend

@MarcoGorelli MarcoGorelli added enhancement New feature or request internal labels Feb 2, 2025
@MarcoGorelli MarcoGorelli changed the title Some more polars optional work feat: allow for tests to be run without Polars installed, unify exceptions more Feb 2, 2025
@MarcoGorelli
Copy link
Member

ah, i tried unifying exceptions whilst we're here, but that got a bit out of hand - will revert and keep this to the original scope

@mgorny
Copy link
Author

mgorny commented Feb 2, 2025

I'm sorry for not taking it further — it's been a busy week.

@MarcoGorelli
Copy link
Member

no worries at all! will pick this up again for the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request internal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants