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

Fixes max, min, prod & sum for keyed tables #19

Merged
merged 3 commits into from
Jan 19, 2024

Conversation

neutropolis
Copy link
Contributor

@neutropolis neutropolis commented Jan 17, 2024

Bug Fix

What is the bug?

If possible please include a summary and reproducible use-case below:
Several functions from the Pandas API raise a length error while working over the default axis.

>>> import pykx as kx
>>> t = kx.q('([a:1 2]b:3 4;c:5 6)')
>>> t
pykx.KeyedTable(pykx.q('
a| b c
-| ---
1| 3 5
2| 4 6
'))
>>> t.max()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/j/Github/hablapps/pykx/pykx-dev/lib/python3.9/site-packages/pykx/pandas_api/__init__.py", line 57, in return_va
l
    res = func(*args, **kwargs)
  File "/Users/j/Github/hablapps/pykx/pykx-dev/lib/python3.9/site-packages/pykx/pandas_api/pandas_meta.py", line 80, in inner
    return q('{[x; y] y!x}', res, cols)
  File "/Users/j/Github/hablapps/pykx/pykx-dev/lib/python3.9/site-packages/pykx/embedded_q.py", line 226, in __call__
    return factory(result, False)
  File "pykx/_wrappers.pyx", line 507, in pykx._wrappers._factory
  File "pykx/_wrappers.pyx", line 500, in pykx._wrappers.factory
pykx.exceptions.QError: length

How does the change fix it?

I think that the simplest fix is to reorder the calculation of the cols variable at the preparse_computations general method. By doing so, we get rid of the redundant keys and the result is consistent with @convert_result. This simple change fixes all max, min, prod and sum (and also any and all) when invoked with a keyed table and default axis.

Code

  • Is code production ready (no stub/test functions, hardcoded IP/ tables/ hostnames, etc.)?

Testing

  • Have unit tests been created or existing ones updated to catch this bug in the future?
  • Has test coverage remained the same or improved?

@neutropolis neutropolis changed the title Reorders cols calculation at preparse_computations Fixes max, min, prod & sum for keyed tables Jan 17, 2024
@github-actions github-actions bot added the tests label Jan 17, 2024
tests/test_pandas_api.py Outdated Show resolved Hide resolved
@neutropolis neutropolis requested a review from rianoc-kx January 17, 2024 16:53
@rianoc-kx rianoc-kx requested a review from cmccarthy1 January 17, 2024 18:13
@cmccarthy1 cmccarthy1 merged commit 720bb41 into KxSystems:main Jan 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants