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

Upgrade nbqa, add config options for black #4135

Merged
merged 1 commit into from
Sep 26, 2020

Conversation

MarcoGorelli
Copy link
Contributor

Ahead of #4095

@MarcoGorelli
Copy link
Contributor Author

@AlexAndorra it seems to me that black_nbconvert doesn't respect the line length from pyproject.toml, e.g. if we have

    average_disasters[i] = np.mean(np.where(idx, trace["early_rate"], trace["late_rate"]))

(90 characters long) it'll reformat it to

    average_disasters[i] = np.mean(
        np.where(idx, trace["early_rate"], trace["late_rate"])
    )

Is that OK, or would you prefer to use the line length from pyproject.toml? (the latter is what I've gone for in this PR)

@codecov
Copy link

codecov bot commented Sep 26, 2020

Codecov Report

Merging #4135 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4135   +/-   ##
=======================================
  Coverage   88.75%   88.75%           
=======================================
  Files          89       89           
  Lines       14037    14037           
=======================================
  Hits        12458    12458           
  Misses       1579     1579           

@AlexAndorra
Copy link
Contributor

Yeah it should use the line length from pyproject.toml IMO

Copy link
Contributor

@AlexAndorra AlexAndorra left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @MarcoGorelli !
Does this mean people should use nbqa to blacken NBs now, instead of black_nbconvert?

@MarcoGorelli
Copy link
Contributor Author

Does this mean people should use nbqa to blacken NBs now, instead of black_nbconvert?

If you want pyproject.toml configs to be respected, then I think that would be better - I'll update the wiki if you want this

@AlexAndorra AlexAndorra mentioned this pull request Sep 26, 2020
@AlexAndorra
Copy link
Contributor

AlexAndorra commented Sep 26, 2020

Ok, let's do that then! Indeed, thanks for updating the wiki 😉
I'll wait to merge this PR, as I think you may have picked up some of these changes in #4136 -- will merge once that's cleared

@AlexAndorra AlexAndorra merged commit ce152fa into pymc-devs:master Sep 26, 2020
@MarcoGorelli MarcoGorelli deleted the upgrade-nbqa branch September 26, 2020 16:24
@AlexAndorra
Copy link
Contributor

@MarcoGorelli, since this is merged now, do you mind updating the NB style guide with the changes please?

@MarcoGorelli
Copy link
Contributor Author

Hey @AlexAndorra - sorry for the delay, I've updated the wiki

@AlexAndorra
Copy link
Contributor

Perfect, thanks a lot @MarcoGorelli !
"This step will eventually be merged in with step 2" --> exciting 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants