-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Run clang-format on objToJSON #28144
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
Failed due to the reorder of imports. Quite a few options outlined here: https://stackoverflow.com/a/37999072/621736 The easiest of which is to turn off the sort-includes flag (though there are more ideal alternatives as well) I guess for now looking for feedback on style and can mess with getting build to work later |
Option here was |
No opinion on this. |
Cool - I'll probably merge later today just to make the file more readable as I've been looking at it |
@@ -242,7 +240,8 @@ static PyObject *get_values(PyObject *obj) { | |||
values = PyObject_CallMethod(values, "to_numpy", NULL); | |||
} | |||
|
|||
if (!is_sparse_array(values) && PyObject_HasAttrString(values, "values")) { | |||
if (!is_sparse_array(values) && | |||
PyObject_HasAttrString(values, "values")) { |
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.
will it complain if an extra indentation goes in here?
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.
Would have to set up a CI rule but clang-format works in the same way as black. It's very opinionated and reformats things for you, so if you tried to add an extra indent and ran the tool it would dedent
Sounds good to me.
…On Mon, Aug 26, 2019 at 9:29 AM William Ayd ***@***.***> wrote:
Cool - I'll probably merge later today just to make the file more readable
as I've been looking at it
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#28144?email_source=notifications&email_token=AAKAOIQ77INXCVS6IU4YCJTQGPSC5A5CNFSM4IPKOOZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5ERHXI#issuecomment-524882909>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAKAOIVYUQ6X5DY5SMNACWDQGPSC5ANCNFSM4IPKOOZA>
.
|
POC for #28143
@TomAugspurger may have interest