Update kdb/tick Syntax to Latest Versions (Specifically 4.1) #1
Conversation
Additionally, I've made some minor changes resulting in code that is more concise.
chraberturas
requested review from
neutropolis,
nipsn,
tortolavivo23 and
Kokechacho
|
|
||
| add:{$[(count w x)>i:w[x;;0]?.z.w;.[`.u.w;(x;i;1);union;y];w[x],:enlist(.z.w;y)];(x;$[99=type v:value x;sel[v]y;@[0#v;`sym;`g#]])} | ||
|
|
||
| sub:{if[x~`;:sub[;y]each t];if[not x in t;'x];del[x].z.w;add[x;y]} | ||
| sub:{[x:{$[not x in t,`;'x;x]};y]if[x~`;:sub[;y]each t];del[x].z.w;add[x;y]} |
There was a problem hiding this comment.
What is the purpose to add
,`
at the end of the condition?
There was a problem hiding this comment.
From what I have been able to guess, since the ifs' are switched, you need to check if x is the null symbol on the first condition as well, and this can be achieved using in, like it has been done here. Still, I don't really see the point of doing this unless we get a speed benefit of some sort.
There was a problem hiding this comment.
Well, I just added it because I tried to incorporate the new syntax as much as possible. I need to measure how fast or slow this is, so once I do that, I'll determine if it's worthwhile or not
|
|
||
| sel:{$[`~y;x;select from x where sym in y]} | ||
| sel:{(select from x where sym in y;x)`~y} |
There was a problem hiding this comment.
If we are not getting a performance bonus from the new implementation I think I would revert back to its previous version, since it's a bit confusing. When reading the code, you look for a conditional statement, but you get something else.
There was a problem hiding this comment.
Well, the main reason for this change was because it's shorter :)
I still believe it's quite clear, so I'll keep it until our master @neutropolis makes the decision
There was a problem hiding this comment.
Then, just to drag my point across, should we change all ifs to this syntax?
(then;)cond;I.e:
if[x~`;:sub[;y]each t];
// to
(:sub[;y]each t];)x~`;Haven't tested it yet, but I'll do so whenever I get to download v4.1
There was a problem hiding this comment.
This is not new in 4.1. We are simply indexing. For example, in this case, x~ will return 0b or 1b. The idea is to index and return the first or second position from the list on the right. I believe this is suitable for small and simple cases.
There was a problem hiding this comment.
My point still stands. I personally would keep it as it was, but if we are changing it then we should keep it consistent and change every instance of a conditional statement OR every if/else ($) statement. Let's wait to see what @neutropolis has to say about this.
|
|
||
| add:{$[(count w x)>i:w[x;;0]?.z.w;.[`.u.w;(x;i;1);union;y];w[x],:enlist(.z.w;y)];(x;$[99=type v:value x;sel[v]y;@[0#v;`sym;`g#]])} | ||
|
|
||
| sub:{if[x~`;:sub[;y]each t];if[not x in t;'x];del[x].z.w;add[x;y]} | ||
| sub:{[x:{$[not x in t,`;'x;x]};y]if[x~`;:sub[;y]each t];del[x].z.w;add[x;y]} |
There was a problem hiding this comment.
From what I have been able to guess, since the ifs' are switched, you need to check if x is the null symbol on the first condition as well, and this can be achieved using in, like it has been done here. Still, I don't really see the point of doing this unless we get a speed benefit of some sort.
…as discussed in PR
…as discussed in PR
…pps/kdb-tick into chraberturas/migration_4.1
…pps/kdb-tick into chraberturas/migration_4.1
Description
This PR updates the syntax of the kdb/tick library to adapt latest changes in native language.
Changes Made