-
-
Notifications
You must be signed in to change notification settings - Fork 312
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
Implement minimal coerce
and corresponding tests
#1772
Conversation
@datajoely FYI I ended up going down the rabbit hole you mentioned myself: However, I found the Ibis |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## ibis-dev #1772 +/- ##
============================================
- Coverage 94.29% 92.78% -1.51%
============================================
Files 91 130 +39
Lines 7024 9458 +2434
============================================
+ Hits 6623 8776 +2153
- Misses 401 682 +281 ☔ View full report in Codecov by Sentry. |
b0c7bce
to
f06eaa5
Compare
f06eaa5
to
fa36b41
Compare
fa36b41
to
666e916
Compare
Oh, I forgot to include #1766, even though this was build on top of that, hence the failures. Since that's merged, I just rebased onto |
Signed-off-by: Deepyaman Datta <[email protected]>
666e916
to
c48487a
Compare
Signed-off-by: Deepyaman Datta <[email protected]>
CI issue fixed, should be good to go! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM @deepyaman! Just have a quick question about the test
"""Get a strategy for an Ibis table of a given dtype.""" | ||
return memtables( | ||
cols=2, | ||
allowed_dtypes=dtype.to_polars(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the conversion to polars dtype?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/deepyaman/pandera/blob/45252183baab6b91416f7373a302ca40979c1e55/tests/ibis/test_ibis_dtypes.py#L30 arguments are passed directly to dataframes
strategy from Polars, and then I'm just constructing the ibis.memtable
from a Polars dataframe.
* Implement minimal `coerce` and corresponding tests Signed-off-by: Deepyaman Datta <[email protected]> * Do not exclude 3.9 in Ibis CI, and regenerate reqs Signed-off-by: Deepyaman Datta <[email protected]> --------- Signed-off-by: Deepyaman Datta <[email protected]>
* Implement minimal `coerce` and corresponding tests Signed-off-by: Deepyaman Datta <[email protected]> * Do not exclude 3.9 in Ibis CI, and regenerate reqs Signed-off-by: Deepyaman Datta <[email protected]> --------- Signed-off-by: Deepyaman Datta <[email protected]>
This builds on top of #1766 and serves to:
coerce
functionality