Add count and add prefix and sufix implementation #12

marcosvm13 · 2023-12-18T12:48:56Z

Feature

Please insert link to associated issue here:

What does this change introduce?

General

Code

Has all temporary code used during development been removed?
Has all commented out (unused) code been removed?
Where reasonable have you ensured there is no duplication of existing code?
If applicable for your use-case have you ensured that the code is performant?

Testing

Have unit tests been created or existing ones updated to test this new functionality?

Documentation

Has documentation been added for all public code?
Has a release note been included for the new feature?
Has any documentation which would benefit from this feature been updated to use the most up to date functionality?
If a new class has been added has a documentation stub .md file associated with it been created?
If any documentation page has been created has it been added to mkdocs.yml
Have you checked your changes with a spell checker? (US English)

docs/user-guide/advanced/Pandas_API.ipynb

tests/test_pandas_api.py

neutropolis · 2023-12-19T06:19:43Z

tests/test_pandas_api.py

+    pcount = df.count(axis=1)
+
+    print(pcount)
+    assert int(qcount[0]) == int(pcount[0])


Why don't we fully compare qcount and pcount instead of just checking the very first index?

This is the reason we have to decouple pandas cases and q: In accordance with the correspondence between q nulls and Python nulls established in the .pd conversion, the empty string is regarded as a null value for columns of type symbol.

neutropolis · 2023-12-19T06:19:46Z

tests/test_pandas_api.py

+
+    print(pcount)
+    assert int(qcount[0]) == int(pcount[0])
+    assert int(qcount[1]) == 1


I'm not sure if this adds value, since I think it's ok to assume that the pandas implementation is correct.

The same as before, in the case of symbols we have to write the case by hand because the conversion is not done automatically. I could create an equivalent Pandas table and not use the automatic switch from Pandas to pykx. But perhaps this way the special case becomes clearer. What you prefer.

neutropolis · 2023-12-19T06:20:30Z

tests/test_pandas_api.py

+    qcount = tab.count().py()
+    pcount = df.count()
+
+    assert int(qcount["k1"]) == int(pcount["k1"])


Again, why not checking the output for both columns k1 and k2?

k2 is not numeric. In this case we could try to test the two objects. But count is a series object (a column table) and the translation does not seem direct to pykx

neutropolis · 2023-12-19T06:21:00Z

tests/test_pandas_api.py

+    pcount = df.count()
+
+    assert int(qcount["k1"]) == int(pcount["k1"])
+    assert int(qcount["k2"]) == 3


Similar to a previous comment, is this really adding value to the test?

tests/test_pandas_api.py

neutropolis · 2023-12-19T06:35:49Z

tests/test_pandas_api.py

@@ -2029,3 +2029,136 @@ def test_keyed_loc_fixes(q):
        mkt[['k1', 'y']]
    with pytest.raises(KeyError):
        mkt['k1']
+
+
+def test_pandas_count(q):


We should supply a case where the numeric_only flag is enabled.

There is a case where numeric_only is added

tests/test_pandas_api.py

neutropolis · 2023-12-19T07:05:05Z

tests/test_pandas_api.py

+        assert np.isnan(p_m[c]) == np.isnan(q_m[c].py())
+
+    q['tab'] = kx.toq(df)
+    tab = q('1!`idx xcols update idx: til count tab from tab')


Suggested change

tab = q('1!`idx xcols update idx: til count tab from tab')

tab = q('`idx xkey update idx:i from tab')

Addition of sensible default

Add count and add prefix and sufix implementation

6eaee34

github-actions bot added documentation Improvements or additions to documentation python tests labels Dec 18, 2023

marcosvm13 and others added 2 commits December 18, 2023 13:53

Update version Pandas API Notebook

f9f396f

Added skew and std implementations

531fc85

neutropolis reviewed Dec 18, 2023

View reviewed changes

docs/user-guide/advanced/Pandas_API.ipynb Outdated Show resolved Hide resolved

docs/user-guide/advanced/Pandas_API.ipynb Outdated Show resolved Hide resolved

fix typo

27f685f

neutropolis reviewed Dec 19, 2023

View reviewed changes

Jesús López-González and others added 6 commits December 19, 2023 08:56

Extends test cases for skew

91caa66

fix problems in test pandas API

348917b

Applied comment for std function

6127e3b

Changed value function call to q.value()

a1ac6d8

Add error validation to test

8e58d37

Update typos in documentation

689100c

neutropolis closed this Dec 21, 2023

chraberturas pushed a commit that referenced this pull request Jan 16, 2024

Merge pull request #12 from KxSystems/KXI-32432

d28e93a

Addition of sensible default

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add count and add prefix and sufix implementation #12

Add count and add prefix and sufix implementation #12

marcosvm13 commented Dec 18, 2023

neutropolis Dec 19, 2023

marcosvm13 Dec 19, 2023

neutropolis Dec 19, 2023

marcosvm13 Dec 19, 2023

neutropolis Dec 19, 2023

marcosvm13 Dec 19, 2023

neutropolis Dec 19, 2023

neutropolis Dec 19, 2023

marcosvm13 Dec 19, 2023

neutropolis Dec 19, 2023 •

edited

Loading

	tab = q('1!`idx xcols update idx: til count tab from tab')
	tab = q('`idx xkey update idx:i from tab')

Add count and add prefix and sufix implementation #12

Add count and add prefix and sufix implementation #12

Conversation

marcosvm13 commented Dec 18, 2023

Feature

What does this change introduce?

General

Code

Testing

Documentation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neutropolis Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

neutropolis Dec 19, 2023 •

edited

Loading