-
Notifications
You must be signed in to change notification settings - Fork 2
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
Refactor q code. pflake8 fixes. #31
Refactor q code. pflake8 fixes. #31
Conversation
@rianoc-kx thanks for the PR! Sorry about the linting errors, but I was unable to get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the q-ish style is more readable. In fact, while reviewing, I was finding it more difficult to understand our own code than your proposed changes. When are you planing to do the refactoring? We are working in many other functions of the Pandas API currently, and I think it would be great if we had access to such code to make our functions homogeneous with yours and avoid further refactorings of our code by your side. Thanks!
t = q('''{[s;t] | ||
c:$[99h~type t;cols value@;cols] t; | ||
(c!`$string[c],\\:string s) xcol t | ||
}''', suffix, t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
t = q(f'{{(c!`$string[x],/:string c:{c_str} y)xcol y}}', prefix, t) | ||
t = q('''{[s;t] | ||
c:$[99h~type t;cols value@;cols] t; | ||
(c!`$string[s],/:string[c]) xcol t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not string c
to keep it consistent with add_suffix
? I ask it since we want to get used to your coding style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I agree - I missed that
m:{(sum (x - avg x) xexp y) % count x}; | ||
g1:{[m;x]m:m[x]; m[3] % m[2] xexp 3%2}[m]; | ||
(g1 each row) * {sqrt[n * n-1] % neg[2] + n:count x} each row | ||
}''', res), cols) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
1~ddof;sdev; | ||
{[ddof;x] avg sqrt (sum xexp[x-avg x;2]) % count[x]-ddof}ddof]; | ||
axis_keys!d each tab | ||
}''', tab, axis, ddof, axis_keys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
cbff0f2
into
hablapps:feature/prefix_suffix_std_skew_count
pflake8
(whitespace etc.)