-
Notifications
You must be signed in to change notification settings - Fork 300
Update black #3864
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
Update black #3864
Conversation
trexfeathers
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.
In some places the new formatting looks really horrible, is less readable and seems gratuitous - see the specific sections I have marked. Is there a configuration we can use to make Black less zealous?
| ( | ||
| "first_validity_time", | ||
| ( | ||
| 21, | ||
| 22, | ||
| 23, | ||
| 24, | ||
| 25, | ||
| 26, | ||
| 27, | ||
| ), | ||
| ), | ||
| ( | ||
| "last_validity_time", | ||
| ( | ||
| 28, | ||
| 29, | ||
| 30, | ||
| 31, | ||
| 32, | ||
| 33, | ||
| 34, | ||
| ), | ||
| ), | ||
| ( | ||
| "misc_validity_time", | ||
| ( | ||
| 35, | ||
| 36, | ||
| 37, | ||
| 38, | ||
| 39, | ||
| 40, | ||
| 41, | ||
| ), | ||
| ), | ||
| ( | ||
| "integer_constants", | ||
| ( | ||
| 100, | ||
| 101, | ||
| ), | ||
| ), | ||
| ( | ||
| "real_constants", | ||
| ( | ||
| 105, | ||
| 106, | ||
| ), | ||
| ), | ||
| ( | ||
| "level_dependent_constants", | ||
| ( | ||
| 110, | ||
| 111, | ||
| 112, | ||
| ), | ||
| ), | ||
| ( | ||
| "row_dependent_constants", | ||
| ( | ||
| 115, | ||
| 116, | ||
| 117, | ||
| ), | ||
| ), | ||
| ( | ||
| "column_dependent_constants", | ||
| ( | ||
| 120, | ||
| 121, | ||
| 122, | ||
| ), | ||
| ), | ||
| ( | ||
| "fields_of_constants", | ||
| ( | ||
| 125, | ||
| 126, | ||
| 127, | ||
| ), | ||
| ), | ||
| ( | ||
| "extra_constants", | ||
| ( | ||
| 130, | ||
| 131, | ||
| ), | ||
| ), | ||
| ( | ||
| "temp_historyfile", | ||
| ( | ||
| 135, | ||
| 136, | ||
| ), | ||
| ), | ||
| ( | ||
| "compressed_field_index1", | ||
| ( | ||
| 140, | ||
| 141, | ||
| ), | ||
| ), | ||
| ( | ||
| "compressed_field_index2", | ||
| ( | ||
| 142, | ||
| 143, | ||
| ), | ||
| ), | ||
| ( | ||
| "compressed_field_index3", | ||
| ( | ||
| 144, | ||
| 145, | ||
| ), | ||
| ), | ||
| ( | ||
| "lookup_table", | ||
| ( | ||
| 150, | ||
| 151, | ||
| 152, | ||
| ), | ||
| ), | ||
| ("total_prognostic_fields", (153,)), | ||
| ("data", (160, 161, 162,)), | ||
| ( | ||
| "data", | ||
| ( | ||
| 160, | ||
| 161, | ||
| 162, | ||
| ), | ||
| ), |
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.
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.
😱
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 know, right...
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.
Almost wondering if it's a bug in black? Would anyone really choose this?
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.
@bjlittle will likely be able to explain all once he pushes the fixing commit. He does know what's wrong
| ( | ||
| "lbrsvd", | ||
| ( | ||
| 34, | ||
| 35, | ||
| 36, | ||
| 37, | ||
| ), | ||
| ), | ||
| ("lbsrce", (38,)), | ||
| ("lbuser", (39, 40, 41, 42, 43, 44, 45,)), | ||
| ("brsvd", (46, 47, 48, 49,)), | ||
| ( | ||
| "lbuser", | ||
| ( | ||
| 39, | ||
| 40, | ||
| 41, | ||
| 42, | ||
| 43, | ||
| 44, | ||
| 45, | ||
| ), | ||
| ), | ||
| ( | ||
| "brsvd", | ||
| ( | ||
| 46, | ||
| 47, | ||
| 48, | ||
| 49, | ||
| ), | ||
| ), |
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.
| ( | ||
| "lbrsvd", | ||
| ( | ||
| 34, | ||
| 35, | ||
| 36, | ||
| 37, | ||
| ), | ||
| ), | ||
| ("lbsrce", (38,)), | ||
| ("lbuser", (39, 40, 41, 42, 43, 44, 45,)), | ||
| ("brsvd", (46, 47, 48, 49,)), | ||
| ( | ||
| "lbuser", | ||
| ( | ||
| 39, | ||
| 40, | ||
| 41, | ||
| 42, | ||
| 43, | ||
| 44, | ||
| 45, | ||
| ), | ||
| ), | ||
| ( | ||
| "brsvd", | ||
| ( | ||
| 46, | ||
| 47, | ||
| 48, | ||
| 49, | ||
| ), | ||
| ), |
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.
| np.logical_and( | ||
| mask_invert[ | ||
| :-1, | ||
| ], | ||
| diffs_along_y, | ||
| ) |
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.
| lat_cube = next( | ||
| self.cube.slices( | ||
| [ | ||
| "grid_latitude", | ||
| ] | ||
| ) | ||
| ) |
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.
| points=[ | ||
| 15, | ||
| ], |
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.
| len( | ||
| set(test_date_ints), | ||
| ), |
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.
| self.assertEqual( | ||
| shape, | ||
| ( | ||
| 2, | ||
| 3, | ||
| ), | ||
| ) |
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.
| self.assertEqual( | ||
| shape, | ||
| ( | ||
| 2, | ||
| 3, | ||
| ), | ||
| ) |
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.
| Index[ | ||
| (2, 0, 1), | ||
| ], |
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.
|
@trexfeathers Yeah, I tend to agree... I am slightly upset with its choices... I'll tinker under the hood and experiment a little with what controls black provides to see if it alleviates these changes. |
|
@trexfeathers I'm closing this in favour of #3866... it just make life easier 😉 |
🚀 Pull Request
Description
This PR bumps the version of the
blackpre-commit hook, and adds format changes to files for the new version standard.Consult Iris pull request check list