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

Setup project to support multiple versions of polars #72

Merged
merged 6 commits into from
Nov 5, 2024

Conversation

santiago-roig
Copy link
Collaborator

@santiago-roig santiago-roig commented Oct 31, 2024

This MR simply allows any codebase which uses pyoframe to specify which version of polars they wish to use. In our case we are trying to migrate to version 1.12, but currently use 0.20. One thing to consider is that I don't allow the polars dependency to increase beyond the minor version 1.12. I've seen in their release logs that they historically have introduced deprecated API calls between minor bumps which is curious. Therefore I think with each update we'll need to refer to the polars update guide and determine if we can push the upper bound on the polars version.

@santiago-roig
Copy link
Collaborator Author

Investigating failing tests. When running locally I had old dependencies so I did not catch failure before MR.

Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.65%. Comparing base (522f48d) to head (592c8f3).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #72      +/-   ##
==========================================
+ Coverage   94.61%   94.65%   +0.03%     
==========================================
  Files          13       13              
  Lines        1431     1441      +10     
==========================================
+ Hits         1354     1364      +10     
  Misses         77       77              
Flag Coverage Δ
smart-tests 94.65% <100.00%> (+0.03%) ⬆️

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.

Copy link
Member

@staadecker staadecker left a comment

Choose a reason for hiding this comment

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

Looks generally good to me, see my comments. Did you do a search on the codebase to make sure we're not missing other .drop() or .join() statements that need updating?

src/pyoframe/constants.py Outdated Show resolved Hide resolved
src/pyoframe/core.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@santiago-roig
Copy link
Collaborator Author

Looks generally good to me, see my comments. Did you do a search on the codebase to make sure we're not missing other .drop() or .join() statements that need updating?

Will double check one more time and clean up code based on comments. Thank you sir for the feedback.

@santiago-roig santiago-roig enabled auto-merge (squash) November 5, 2024 13:33
@santiago-roig santiago-roig dismissed staadecker’s stale review November 5, 2024 13:34

All comments have been addressed. Not sure why it's still showing one request.

@santiago-roig santiago-roig merged commit b1ea23f into main Nov 5, 2024
5 checks passed
@santiago-roig santiago-roig deleted the sr/bump-polars branch November 5, 2024 13:34
@staadecker
Copy link
Member

Very cool your matrix testing 😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants