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 black to 21.7b0 #116

Merged
merged 1 commit into from
Aug 9, 2021
Merged

Conversation

jalaziz
Copy link
Contributor

@jalaziz jalaziz commented Aug 3, 2021

Upgrade to the latest release of black.

This allows downstream projects like Github's super-linter to upgrade black as well.

I am seeing some test failures, but haven't been able to pin down what about the black upgrade is causing the breakage.

@mbhall88
Copy link
Member

mbhall88 commented Aug 3, 2021

@bricoletc any idea why changing the black version would cause these 2 grammar tests to fail?

Upgrade to the latest release of black.
@jalaziz
Copy link
Contributor Author

jalaziz commented Aug 3, 2021

@mbhall88 seems like it was an issue with the dev dependencies. By only upgrading black (poetry update --no-dev black) all the tests now pass.

@jalaziz
Copy link
Contributor Author

jalaziz commented Aug 3, 2021

If do a full update, I get:

Package operations: 5 installs, 26 updates, 1 removal

  • Removing chardet (4.0.0)
  • Updating decorator (4.4.2 -> 5.0.9)
  • Updating six (1.15.0 -> 1.16.0)
  • Updating attrs (20.3.0 -> 21.2.0)
  • Updating docutils (0.16 -> 0.17.1)
  • Updating pyrsistent (0.17.3 -> 0.18.0)
  • Updating smmap (3.0.5 -> 4.0.0)
  • Updating certifi (2020.12.5 -> 2021.5.30)
  • Installing charset-normalizer (2.0.4)
  • Updating gitdb (4.0.5 -> 4.0.7)
  • Updating idna (2.10 -> 3.2)
  • Updating more-itertools (8.7.0 -> 8.8.0)
  • Updating packaging (20.9 -> 21.0)
  • Installing typing-extensions (3.10.0.0)
  • Updating urllib3 (1.26.3 -> 1.26.6)
  • Updating configargparse (1.3 -> 1.5.1)
  • Installing connection-pool (0.0.3)
  • Updating gitpython (3.1.14 -> 3.1.20)
  • Updating nbformat (5.1.2 -> 5.1.3)
  • Updating pathspec (0.8.1 -> 0.9.0)
  • Updating pycodestyle (2.6.0 -> 2.7.0)
  • Updating pyflakes (2.2.0 -> 2.3.1)
  • Updating regex (2020.11.13 -> 2021.7.6)
  • Updating requests (2.25.1 -> 2.26.0)
  • Updating smart-open (4.2.0 -> 5.1.0)
  • Installing stopit (1.1.2)
  • Installing tabulate (0.8.9)
  • Updating zipp (3.4.0 -> 3.5.0)
  • Updating flake8 (3.8.4 -> 3.9.2)
  • Updating isort (5.7.0 -> 5.9.3)
  • Updating pytest-cov (2.11.1 -> 2.12.1)
  • Updating snakemake (6.0.1 -> 6.6.1)

Seems like the snakemake update may be the reason for the grammar tests failing. If I pin it to 6.0.1, the grammar tests no longer fail.

@jalaziz
Copy link
Contributor Author

jalaziz commented Aug 3, 2021

The one non-grammar test seems to fail because of the pathspec update. If I downgrade pathspec and snakemake all tests pass.

Not quite sure why updating pathspec would cause a newline to be removed, but that's a battle for another day. The PR as of my most recent push should be pass all tests now.

@jalaziz
Copy link
Contributor Author

jalaziz commented Aug 6, 2021

@mbhall88 Just wanted to follow up and see what I could do to unblock this?

@mbhall88
Copy link
Member

mbhall88 commented Aug 6, 2021

Waiting on input from @bricoletc

@codecov
Copy link

codecov bot commented Aug 6, 2021

Codecov Report

Merging #116 (c8a4a99) into master (c81a570) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #116   +/-   ##
=======================================
  Coverage   98.21%   98.21%           
=======================================
  Files          12       12           
  Lines         951      951           
  Branches      174      174           
=======================================
  Hits          934      934           
  Misses         10       10           
  Partials        7        7           
Flag Coverage Δ
unittests 98.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@bricoletc
Copy link
Collaborator

Looks good to me, though now this would make us require python 3.6.2 not 3.6.1 @mbhall88. I guess I'm ok with that though snakemake supports 3.5+

@jalaziz seems like for the super-linter issue, where black version was being pinned by snakefmt, is solved? So do you still need this black version update?

Anyway, I'm happy doing this update as it doesn't break anything

@jalaziz
Copy link
Contributor Author

jalaziz commented Aug 8, 2021

Looks good to me, though now this would make us require python 3.6.2 not 3.6.1 @mbhall88. I guess I'm ok with that though snakemake supports 3.5+

@jalaziz seems like for the super-linter issue, where black version was being pinned by snakefmt, is solved? So do you still need this black version update?

Anyway, I'm happy doing this update as it doesn't break anything

@bricoletc Unfortunately the super-linter fix didn't actually work and had to be reverted.

@mbhall88 mbhall88 merged commit 91603a5 into snakemake:master Aug 9, 2021
@jalaziz jalaziz deleted the upgrade-black branch August 9, 2021 04:04
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.

3 participants