Skip to content

Conversation

@mutricyl
Copy link
Contributor

  • Closes follow up from #237 & #239 #240
  • Executed pre-commit run --all-files with no errors
  • The change is fully covered by automated unit tests: hopefully with latest version of pint
  • Documented in docs/ as appropriate
  • Added an entry to the CHANGES file

need to wait for new release of pint after hgrecco/pint#2036

@andrewgsavage
Copy link
Collaborator

self = <pint_pandas.testsuite.test_issues.TestIssue80 object at 0x7f1f27ce2440>
reduction = 'min'

    @pytest.mark.parametrize(
        "reduction",
        ["min", "max", "sum", "mean", "median"],
    )
    def test_reductions(self, reduction):
        # before the fix, those reductions could be *very* slow. Fail early.
        for n in [10_000, 1_000_000]:
            s_pint = self._make_df(n)["time"]
            s = self._make_df(n, pint_units=False)["time"]
    
            tp = self._timeit(getattr(s_pint, reduction)).to("ms")
            t = self._timeit(getattr(s, reduction)).to("ms")
    
            if t > 0:
>               assert tp <= 5 * t
E               AssertionError: assert <Quantity(3.80[40](https://github.com/hgrecco/pint-pandas/actions/runs/9909151676/job/27376580214?pr=244#step:11:41)94, 'millisecond')> <= (5 * <Quantity(0.648044, 'millisecond')>)

pint_pandas/testsuite/test_issues.py:1[41](https://github.com/hgrecco/pint-pandas/actions/runs/9909151676/job/27376580214?pr=244#step:11:42): AssertionError

Try changing this to 10 * t

>               assert tp <= 5 * t

@andrewgsavage andrewgsavage merged commit fcd41c2 into hgrecco:master Jul 13, 2024
@andrewgsavage
Copy link
Collaborator

very nice, thanks

@mutricyl
Copy link
Contributor Author

Thanks for your help and swift review/merge. Long journey to get eval to work with pint-pandas!! 😃 need to get some rest now😪 (vacation time for me).

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.

follow up from #237 & #239

2 participants