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

chore(python): Suggest allow_null as replacement #17969

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

deepyaman
Copy link
Contributor

@deepyaman deepyaman commented Jul 31, 2024

Trivial fix as I'm going through the Hypothesis strategy implementation

@github-actions github-actions bot added internal An internal refactor or improvement python Related to Python Polars labels Jul 31, 2024
@deepyaman deepyaman closed this Jul 31, 2024
@deepyaman deepyaman deleted the patch-1 branch July 31, 2024 22:48
@deepyaman deepyaman restored the patch-1 branch July 31, 2024 22:59
@deepyaman deepyaman reopened this Jul 31, 2024
Copy link

codecov bot commented Jul 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.43%. Comparing base (6065465) to head (59a4966).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #17969      +/-   ##
==========================================
+ Coverage   80.39%   80.43%   +0.04%     
==========================================
  Files        1494     1494              
  Lines      196480   196480              
  Branches     2817     2817              
==========================================
+ Hits       157957   158041      +84     
+ Misses      38003    37919      -84     
  Partials      520      520              

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

@deepyaman
Copy link
Contributor Author

I second-guessed myself seeing as it's used in https://github.com/unionai-oss/pandera/blob/v0.20.3/tests/polars/test_polars_dtypes.py#L86, but I'm pretty sure Polars is just throwing that argument away. For example:

>>> from polars.testing.parametric import dataframes
>>> dataframes(cols=2, allowed_dtypes=pl.Int64, include_nulls=0.1, min_size=10, max_size=10).example()
shape: (10, 2)
┌──────────────────────┬──────────────────────┐
│ col0                 ┆ col1                 │
│ ---                  ┆ ---                  │
│ i64                  ┆ i64                  │
╞══════════════════════╪══════════════════════╡
│ 59997                ┆ 59997                │
│ null                 ┆ null                 │
│ null                 ┆ null                 │
│ 6002                 ┆ 6002                 │
│ -3366933560069126664 ┆ -3366933560069126664 │
│ null                 ┆ null                 │
│ 53462                ┆ 53462                │
│ null                 ┆ null                 │
│ 40624                ┆ 40624                │
│ null                 ┆ null                 │
└──────────────────────┴──────────────────────┘
>>> dataframes(cols=2, allowed_dtypes=pl.Int64, include_nulls=0, min_size=10, max_size=10).example()
shape: (10, 2)
┌──────────────────────┬────────────────────┐
│ col0                 ┆ col1               │
│ ---                  ┆ ---                │
│ i64                  ┆ i64                │
╞══════════════════════╪════════════════════╡
│ null                 ┆ null               │
│ 65236                ┆ null               │
│ null                 ┆ null               │
│ null                 ┆ null               │
│ -8817020948431784673 ┆ null               │
│ 159                  ┆ -40705             │
│ null                 ┆ null               │
│ null                 ┆ -36725             │
│ -6248418179401011001 ┆ 174977669653484525 │
│ null                 ┆ -7945              │
└──────────────────────┴────────────────────┘
>>> dataframes(cols=2, allowed_dtypes=pl.Int64, allow_null=False, min_size=10, max_size=10).example()
shape: (10, 2)
┌──────────────────────┬──────────────────────┐
│ col0                 ┆ col1                 │
│ ---                  ┆ ---                  │
│ i64                  ┆ i64                  │
╞══════════════════════╪══════════════════════╡
│ -118                 ┆ -118                 │
│ 21080                ┆ 21080                │
│ 3258                 ┆ 3258                 │
│ 105                  ┆ 105                  │
│ 21534                ┆ 21534                │
│ -29                  ┆ -29                  │
│ -24673               ┆ -118                 │
│ -2726383874071525662 ┆ -2726383874071525662 │
│ -38                  ┆ -38                  │
│ 226                  ┆ 226                  │
└──────────────────────┴──────────────────────┘
>>> dataframes(cols=2, allowed_dtypes=pl.Int64, allow_null=True, min_size=10, max_size=10).example()
shape: (10, 2)
┌────────────┬────────────┐
│ col0       ┆ col1       │
│ ---        ┆ ---        │
│ i64        ┆ i64        │
╞════════════╪════════════╡
│ -24923     ┆ -24923     │
│ -14693     ┆ -14693     │
│ 1733       ┆ 1733       │
│ null       ┆ null       │
│ null       ┆ null       │
│ -619839569 ┆ -619839569 │
│ null       ┆ null       │
│ null       ┆ null       │
│ null       ┆ null       │
│ -18        ┆ -18        │
└────────────┴────────────┘
>>> 

@alexander-beedie
Copy link
Collaborator

I second-guessed myself seeing as it's used in https://github.com/unionai-oss/pandera/blob/v0.20.3/tests/polars/test_polars_dtypes.py#L86, but I'm pretty sure Polars is just throwing that argument away.

Yup; that code should likely be updated to use allow_null=True instead 👌
(I see you have a PR pending there).

@alexander-beedie alexander-beedie merged commit c975ec8 into pola-rs:main Aug 1, 2024
32 checks passed
@deepyaman deepyaman deleted the patch-1 branch August 1, 2024 12:01
@deepyaman
Copy link
Contributor Author

deepyaman commented Aug 1, 2024

I second-guessed myself seeing as it's used in https://github.com/unionai-oss/pandera/blob/v0.20.3/tests/polars/test_polars_dtypes.py#L86, but I'm pretty sure Polars is just throwing that argument away.

Yup; that code should likely be updated to use allow_null=True instead 👌 (I see you have a PR pending there).

Yeah, I just deleted the argument, because it already defaults to True. No big rush to fix it there, I guess, since the default is fine and the wrong argument is just ignored, but the PR will resolve it eventually. 🤞

Thanks for confirming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants