Open StijnKas opened 2 months ago
@operdeck on my end, the only tests not passing are the BinAggregator tests (the second and the last). Would you mind having a look at these and seeing if you can figure something out? test_overall_sym_rollup
gives polars.exceptions.ComputeError: get index is out of bounds
. I don't expect Polars V1 to release before EOW next week, but I'd like to have this one ready go for when it does. To test, you can just install latest pdstools, then do pip install --upgrade polars --pre
.
@yusufuyanik1 @operdeck could you help me validate this branch on your workflows and fix any issues you may come across? We should probably get to polars V1 sooner rather than later, given we're only going to fall further behind and run into more issues down the line
@yusufuyanik1 @operdeck Could both of you look at this tomorrow? I will too, let's get it merged so we are semi-up to date again
Attention: Patch coverage is 46.81529%
with 167 lines
in your changes missing coverage. Please review.
Project coverage is 59.68%. Comparing base (
d6cc0c5
) to head (37f761e
). Report is 47 commits behind head on master.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
FYI @yusufuyanik1 , we're also getting a ton of warnings in the health check execution: even things like 'where' being used over 'filter', which I think is a relatively old change. Let's get those out of the way as well
Polars V1 brought quite some breaking changes, this PR gets us ready for when it releases, and would tie our Polars version compatibility to Polars V1.0.x. Given how core Polars is to the functionality of pdstools, we can't always use the latest version, but given that we'd like to stay up to date, we'll tie ourselves to each minor version. Once a new minor version becomes available, we'll retest our changes on the increment and see if we can relieve this requirement.
As part of this PR, the main changes I had to cover were:
StringCache()
no longer took True as an argumentstrict
argument on DataFrame/LazyFrame/Series became more .. strict, so I turned it off in the short-term (this was not critical anywhere anyway, moreso within the testing code)model_summary
suffix
/prefix
onexpr
s are now part of thename
namespaceAlso added some QOL features such as checking Pandoc version & adding TestBook as an optional dependency.