-
-
Notifications
You must be signed in to change notification settings - Fork 365
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
Remove redundant py2 helper code #1316
Conversation
63e6795
to
b0a2da7
Compare
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.
Thank you!
We vendored datashape this fall as it failed to install with Python 3.12, as it hadn't been updated for some time... That does not mean we should have Python 2 code in it.
I haven't set over the test because #1314 is needed for them to pass. I will set them over when the PR is merged.
b0a2da7
to
3f74484
Compare
Co-authored-by: Simon Høxbro Hansen <[email protected]>
Makes sense! I was starting to very gently poke around to see what it might take to get first-class Polars integration, and just happened to spot this 🤔 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1316 +/- ##
==========================================
- Coverage 85.64% 85.63% -0.01%
==========================================
Files 52 51 -1
Lines 11291 11286 -5
==========================================
- Hits 9670 9665 -5
Misses 1621 1621 |
Thank you for the PR 👍 |
Some long-unnecessary py2 → py3
int
andstr
helper stubs were lurking in the codebase.These can be removed to clean things up a bit (with the minor/bonus side-effect of making all the
isinstance
checks that referenced them ~1.5-2x faster, as the helper stubs were both single-element tuples).