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

chore: Run pyupgrade on codebase (3.6+) #1164

Merged
merged 30 commits into from
Nov 5, 2020
Merged

Conversation

kratsg
Copy link
Contributor

@kratsg kratsg commented Nov 5, 2020

Description

Run asottile/pyupgrade on our codebase. Not sure how to implement a CI/github actions for this (@asottile might have suggestions?)

Checklist Before Requesting Reviewer

  • Tests are passing
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
* Run pyupgrade --py36-plus on codebase
* Add pyupgrade to pre-commit hooks
* Add pre-commit.ci status badge to README
* Use f-strings over format() as much as possible
* Thanks to Anthony Sottile (@asottile) for pyupgrade and pre-commit.ci

Co-authored-by: Matthew Feickert <[email protected]>

@kratsg kratsg added the refactor A code change that neither fixes a bug nor adds a feature label Nov 5, 2020
@kratsg kratsg self-assigned this Nov 5, 2020
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@asottile
Copy link

asottile commented Nov 5, 2020

for CI suggestions it looks like you're already using pre-commit so you can add this configuration there

as for running in CI, I've been building https://pre-commit.ci -- if you're interested in the open-but-secret alpha you can set it up at https://results.pre-commit.ci

otherwise if you want to stick to github actions there's https://github.com/pre-commit/action

Copy link
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

Nice. @kratsg Can we also get all these "should be f-string" cases taken care of as well?

@codecov
Copy link

codecov bot commented Nov 5, 2020

Codecov Report

Merging #1164 into master will not change coverage.
The diff coverage is 99.07%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1164   +/-   ##
=======================================
  Coverage   97.18%   97.18%           
=======================================
  Files          63       63           
  Lines        3663     3663           
  Branches      523      522    -1     
=======================================
  Hits         3560     3560           
  Misses         64       64           
  Partials       39       39           
Flag Coverage Δ
unittests 97.18% <99.07%> (ø)

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

Impacted Files Coverage Δ
src/pyhf/utils.py 96.42% <50.00%> (ø)
src/pyhf/cli/infer.py 97.64% <100.00%> (ø)
src/pyhf/cli/patchset.py 100.00% <100.00%> (ø)
src/pyhf/cli/rootio.py 92.15% <100.00%> (ø)
src/pyhf/cli/spec.py 100.00% <100.00%> (ø)
src/pyhf/constraints.py 96.94% <100.00%> (ø)
src/pyhf/events.py 97.29% <100.00%> (ø)
src/pyhf/exceptions/__init__.py 100.00% <100.00%> (ø)
src/pyhf/infer/calculators.py 100.00% <100.00%> (ø)
src/pyhf/interpolators/code0.py 100.00% <100.00%> (ø)
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81c9adb...05795fd. Read the comment docs.

@matthewfeickert
Copy link
Member

as for running in CI, I've been building https://pre-commit.ci -- if you're interested in the open-but-secret alpha you can set it up at https://results.pre-commit.ci

Thanks @asottile! I've gone ahead and setup pyhf on https://pre-commit.ci 👍

@asottile
Copy link

asottile commented Nov 5, 2020

uh hmmm that's interesting -- it's not supposed to double-update so I've got a bug to fix (the recursion detection appears broken!)

@matthewfeickert matthewfeickert added the docs Documentation related label Nov 5, 2020
Copy link
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

@kratsg Actually, if we want to just get the f-string stuff in a seperate PR I'm good with that so that we can just move on with this nice change. :) Thanks for making great stuff @asottile.

@matthewfeickert matthewfeickert added the chore Other changes that don't modify src or test files label Nov 5, 2020
@matthewfeickert matthewfeickert merged commit a3b34a5 into master Nov 5, 2020
@matthewfeickert matthewfeickert deleted the feat/pyupgrade branch November 5, 2020 23:31
matthewfeickert added a commit that referenced this pull request Nov 6, 2020
* Add nbqa to pre-commit hooks
   - Apply nbqa-black and nbqa-pyupgrade
* Add nbqa config options to pyproject.toml
* Run nbqa over all notebooks
* Add --py36-plus option as arg to pyupgrade pre-commit hook
   - Amends PR #1164
Aspyona pushed a commit to Aspyona/pyhf that referenced this pull request Nov 22, 2020
* Run pyupgrade --py36-plus on codebase
* Add pyupgrade to pre-commit hooks
* Add pre-commit.ci status badge to README
* Use f-strings over format() as much as possible
* Thanks to Anthony Sottile (@asottile) for pyupgrade and pre-commit.ci

Co-authored-by: Matthew Feickert <[email protected]>
Aspyona pushed a commit to Aspyona/pyhf that referenced this pull request Nov 22, 2020
* Add nbqa to pre-commit hooks
   - Apply nbqa-black and nbqa-pyupgrade
* Add nbqa config options to pyproject.toml
* Run nbqa over all notebooks
* Add --py36-plus option as arg to pyupgrade pre-commit hook
   - Amends PR scikit-hep#1164
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Other changes that don't modify src or test files docs Documentation related refactor A code change that neither fixes a bug nor adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants