-
Notifications
You must be signed in to change notification settings - Fork 82
Fix partbyenum to enumerate against placeholding symbol #691
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
Conversation
code/processes/idb.q
Outdated
| /- helper function to support queries against the sym column | ||
| maptoint:{[symbol] | ||
| sym?symbol | ||
| sym?`NONE^symbol |
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.
Could we use something different here - NONE seems like it could validly be in an instrument universe
https://finance.yahoo.com/quote/NONE-USD/
Can we do something like `TORQNULLSYMBOL or something
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 am happy to fix this to use some explicitly stated `TORQNULLSYMBOL or similar. Existing code moved null symbols to NONE
code/processes/wdb.q
Outdated
| (sortedlist union tables[`.]) except ignorelist} | ||
|
|
||
| /- function to check for null syms | ||
| checknullsym:{[s] -1 _ `${@[x; where not ((type each x) in (10 -10h));string]} s,(::)} |
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.
What is the purpose of this function?
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.
This was existing logic to check whether a sym was null (old wdb code line 126). I wanted leverage it in new code line 124, and didn't want any duplicate code so moved it into its own function
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 think the name of this function/the comment above is misleading. It's not checking for null syms, it's making sure that regardless of what is passed to it, a list of syms is returned. Can you update the name/comment to something more descriptive of what's actually happening here? Maybe ensuresymlist or something.
jonathonmcmurray
left a comment
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.
One naming issue, otherwise looks good
code/processes/wdb.q
Outdated
| (sortedlist union tables[`.]) except ignorelist} | ||
|
|
||
| /- function to check for null syms | ||
| checknullsym:{[s] -1 _ `${@[x; where not ((type each x) in (10 -10h));string]} s,(::)} |
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 think the name of this function/the comment above is misleading. It's not checking for null syms, it's making sure that regardless of what is passed to it, a list of syms is returned. Can you update the name/comment to something more descriptive of what's actually happening here? Maybe ensuresymlist or something.
PR to resolve #690