-
Notifications
You must be signed in to change notification settings - Fork 46
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
Implement gel.Record type #560
Conversation
It's returned from `client.query_sql()` and represents SQL rows. Currently it supports: * integer indexing - fetch columns by position * string indexing - fetch columns by name * as_dict() method - render record as a dict ToDo: * add __iter__
gel/datatypes/object.c
Outdated
@@ -284,7 +265,7 @@ object_repr(EdgeObject *o) | |||
|
|||
if (_EdgeGeneric_RenderItems(&writer, | |||
(PyObject *)o, o->desc, | |||
o->ob_item, Py_SIZE(o), 1, 0) < 0) | |||
o->ob_item, Py_SIZE(o), 1, 1, 0) < 0) |
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.
You don't really need to fix this now, but this sort of list of boolean arguments ought to either have comments saying what each of the argument names are or should use #defines/enums or flag bits or something. 1, 1, 0
is inscrutable.
This is an existing problem, though, so it's fine to skip fixing it now.
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.
Fixed in Apply better style for passing flags
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.
Much nicer
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.
Looks reasonable. Kind of a lot of duplication, but you said that merging it would be worse?
What's duplicated? Besides the Record codec looking like a stripped down Object codec? Edit: here's the previous attempt 00650b0 |
It's returned from
client.query_sql()
and represents SQL rows.Currently it supports:
ToDo: