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

CLN: isort from attrs_caching.py to eval.py #25060

Closed

Conversation

gasparia405
Copy link
Contributor

  • passes git diff upstream/master -u -- "*.py" | flake8 --diff

More updates from issue #23334:

  • asv_bench/benchmarks/attrs_caching.py
  • asv_bench/benchmarks/binary_ops.py
  • asv_bench/benchmarks/categorical.py
  • asv_bench/benchmarks/ctors.py
  • asv_bench/benchmarks/dtypes.py
  • asv_bench/benchmarks/eval.py

@codecov
Copy link

codecov bot commented Jan 31, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #25060   +/-   ##
=======================================
  Coverage   92.37%   92.37%           
=======================================
  Files         166      166           
  Lines       52403    52403           
=======================================
  Hits        48405    48405           
  Misses       3998     3998
Flag Coverage Δ
#multiple 90.79% <ø> (ø) ⬆️
#single 42.87% <ø> (ø) ⬆️

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 da5f5eb...cc62d21. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 31, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #25060   +/-   ##
=======================================
  Coverage   91.26%   91.26%           
=======================================
  Files         173      173           
  Lines       52968    52968           
=======================================
  Hits        48339    48339           
  Misses       4629     4629
Flag Coverage Δ
#multiple 89.83% <ø> (ø) ⬆️
#single 41.71% <ø> (ø) ⬆️

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 976a2db...689e8bd. Read the comment docs.

@gasparia405
Copy link
Contributor Author

The error I'm getting is ##[error]Benchmarks run with errors in the Checks_and_doc Job > Running benchmarks test. I solved one failing test from the last commit by returning the asv_bench/benchmarks/dtypes.py to what it was before I ran isort. I imagine something similar went wrong this time, but looking through the diffs and the output, I'm not sure where to start looking. Any ideas?

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Jan 31, 2019 via email

@jschendel jschendel added Code Style Code style, linting, code_checks Clean labels Feb 1, 2019
@WillAyd
Copy link
Member

WillAyd commented Feb 28, 2019

@gasparia405 can you merge master? IIRC had a recent PR to fix issue with benchmark tests

@gasparia405
Copy link
Contributor Author

@WillAyd Thanks for the heads up. I've merged with master and tests passing!

@@ -34,4 +36,4 @@ def time_cache_readonly(self):
self.obj.prop


from .pandas_vb_common import setup # noqa: F401
from .pandas_vb_common import setup # noqa: F401 isort:skip
Copy link
Member

Choose a reason for hiding this comment

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

IIRC from other PRs this was moved to the bottom of the benchmark instead of adding the skip. Would rather do the same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had this issue in #24958 where the linting fails if isort:skip isn't included. I'm looking into the other errors now and it looks like two of the benchmarks failed.

@WillAyd
Copy link
Member

WillAyd commented Mar 9, 2019

Hmm the benchmark failures might be related to work done in #25517

@WillAyd
Copy link
Member

WillAyd commented Mar 19, 2019

@gasparia405 can you merge master?

@WillAyd
Copy link
Member

WillAyd commented Apr 10, 2019

Closing as stale ping if you'd like to pick this back up

@WillAyd WillAyd closed this Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants