Skip to content
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

run black and blackdoc #4381

Merged
merged 1 commit into from
Aug 27, 2020
Merged

run black and blackdoc #4381

merged 1 commit into from
Aug 27, 2020

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Aug 27, 2020

black changed their style (format docstrings and trailing comma handling mostly) in their new release. This simply runs that new version of black on the repository.

This is a separate PR so reviewing becomes easier.

Since it is an automatic change, this should be ready to be merged once the CI is green.

@max-sixty
Copy link
Collaborator

Do we need to change the pre-commit versions?

I guess CI pulls latest and so that automatically updates?

@keewis
Copy link
Collaborator Author

keewis commented Aug 27, 2020

Do we need to change the pre-commit versions?

I don't think so, we're using the stable branch so there shouldn't be anything to do (I'm not sure which criteria per-commit uses to trigger updates, though).

The only thing we can change in this PR is the trailing comma, so it would be good to review that.

@max-sixty
Copy link
Collaborator

Ah, sorry, it's only mypy which is pinned.

LGTM!

@keewis
Copy link
Collaborator Author

keewis commented Aug 27, 2020

actually, everything except black and blackdoc is pinned 😁

@max-sixty
Copy link
Collaborator

actually, everything except black and blackdoc is pinned 😁

Do you think they should be? It's going to be a difficult error to debug for folks who don't know the tools well, if there are backward / forward incompatible changes (maybe there aren't any though?)

@max-sixty max-sixty merged commit ce15385 into pydata:master Aug 27, 2020
@keewis keewis deleted the black branch August 27, 2020 14:57
Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just two minor nits.

@@ -56,7 +56,9 @@
)


def broadcast_dimension_size(variables: List[Variable],) -> Dict[Hashable, int]:
def broadcast_dimension_size(
variables: List[Variable],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems unnecessary

Suggested change
variables: List[Variable],
variables: List[Variable]

@@ -322,7 +322,9 @@ def _assert_dataset_invariants(ds: Dataset):
assert isinstance(ds._attrs, (type(None), dict))


def _assert_internal_invariants(xarray_obj: Union[DataArray, Dataset, Variable],):
def _assert_internal_invariants(
xarray_obj: Union[DataArray, Dataset, Variable],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
xarray_obj: Union[DataArray, Dataset, Variable],
xarray_obj: Union[DataArray, Dataset, Variable]

also unnecessary?

@dcherian
Copy link
Contributor

hahaha no worries.

@max-sixty
Copy link
Collaborator

D'oh @dcherian !

@keewis keewis mentioned this pull request Aug 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants