Skip to content

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

Merged
merged 2 commits into from
Aug 26, 2019
Merged

Run clang-format on objToJSON #28144

merged 2 commits into from
Aug 26, 2019

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Aug 25, 2019

POC for #28143

@TomAugspurger may have interest

@WillAyd WillAyd added the Code Style Code style, linting, code_checks label Aug 25, 2019
@WillAyd
Copy link
Member Author

WillAyd commented Aug 25, 2019

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

@WillAyd
Copy link
Member Author

WillAyd commented Aug 26, 2019

Option here was clang-format -sort-includes=0 -i -style="{IndentWidth: 4}" pandas/_libs/src/ujson/python/objToJSON.c

@TomAugspurger
Copy link
Contributor

No opinion on this.

@WillAyd
Copy link
Member Author

WillAyd commented Aug 26, 2019

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")) {
Copy link
Member

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?

Copy link
Member Author

@WillAyd WillAyd Aug 26, 2019

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

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Aug 26, 2019 via email

@WillAyd WillAyd marked this pull request as ready for review August 26, 2019 17:22
@WillAyd WillAyd added this to the 1.0 milestone Aug 26, 2019
@WillAyd WillAyd merged commit bca39a7 into pandas-dev:master Aug 26, 2019
@WillAyd WillAyd deleted the ext-indent branch August 26, 2019 17:54
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants