STYLE: #22885, Moved all setup imports to bottom of benchmark files and added noqa: F401 to each by battlesdev · Pull Request #22947 · pandas-dev/pandas

battlesdev · 2018-10-02T16:17:35Z

closes STYLE: Fix lint error "F811 redefinition of unused 'setup'" in benchmarks #22885
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Following the suggestions of @datapythonista, I moved all the setup imports for the benchmark files to the end of each file and added a # noqa: F401 like so:

from .pandas_vb_common import setup # noqa: F401

Now, flake8 only returns the following two errors in asv_bench:

./benchmarks/strings.py:95:9: E731 do not assign a lambda expression, use a def
./benchmarks/io/excel.py:6:1: F401 '..pandas_vb_common.BaseIO' imported but unused

These may be expected, but I wanted to mention them here just in case they need addressing.

Also, running pytest, I got this error:

usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --strict-data-files
  inifile: /pandas/setup.cfg
  rootdir: /pandas

I found a discussion on it at #22700, but there didn't seem to be a definitive solution. Open to guidance on this.

Please let me know if any corrections are needed. Thanks!

pep8speaks · 2018-10-02T16:17:48Z

Hello @jerodestapa! Thanks for submitting the PR.

There are no PEP8 issues in the file asv_bench/benchmarks/algorithms.py !
There are no PEP8 issues in the file asv_bench/benchmarks/attrs_caching.py !
There are no PEP8 issues in the file asv_bench/benchmarks/binary_ops.py !
There are no PEP8 issues in the file asv_bench/benchmarks/categoricals.py !
There are no PEP8 issues in the file asv_bench/benchmarks/ctors.py !
There are no PEP8 issues in the file asv_bench/benchmarks/eval.py !
There are no PEP8 issues in the file asv_bench/benchmarks/frame_ctor.py !
There are no PEP8 issues in the file asv_bench/benchmarks/frame_methods.py !
There are no PEP8 issues in the file asv_bench/benchmarks/gil.py !
There are no PEP8 issues in the file asv_bench/benchmarks/groupby.py !
There are no PEP8 issues in the file asv_bench/benchmarks/index_object.py !
There are no PEP8 issues in the file asv_bench/benchmarks/indexing.py !
There are no PEP8 issues in the file asv_bench/benchmarks/inference.py !
There are no PEP8 issues in the file asv_bench/benchmarks/io/csv.py !
There are no PEP8 issues in the file asv_bench/benchmarks/io/excel.py !
There are no PEP8 issues in the file asv_bench/benchmarks/io/hdf.py !
There are no PEP8 issues in the file asv_bench/benchmarks/io/json.py !
There are no PEP8 issues in the file asv_bench/benchmarks/io/msgpack.py !
There are no PEP8 issues in the file asv_bench/benchmarks/io/pickle.py !
There are no PEP8 issues in the file asv_bench/benchmarks/io/sql.py !
There are no PEP8 issues in the file asv_bench/benchmarks/io/stata.py !
There are no PEP8 issues in the file asv_bench/benchmarks/join_merge.py !
There are no PEP8 issues in the file asv_bench/benchmarks/multiindex_object.py !
There are no PEP8 issues in the file asv_bench/benchmarks/panel_ctor.py !
There are no PEP8 issues in the file asv_bench/benchmarks/panel_methods.py !
There are no PEP8 issues in the file asv_bench/benchmarks/plotting.py !
There are no PEP8 issues in the file asv_bench/benchmarks/reindex.py !
There are no PEP8 issues in the file asv_bench/benchmarks/replace.py !
There are no PEP8 issues in the file asv_bench/benchmarks/reshape.py !
There are no PEP8 issues in the file asv_bench/benchmarks/rolling.py !
There are no PEP8 issues in the file asv_bench/benchmarks/series_methods.py !
There are no PEP8 issues in the file asv_bench/benchmarks/sparse.py !
There are no PEP8 issues in the file asv_bench/benchmarks/stat_ops.py !
There are no PEP8 issues in the file asv_bench/benchmarks/timeseries.py !

datapythonista

lgtm. The approach is not ideal of course, but I think it's the best we can do.

Once this and #22863 are merged, we can activate the linting of the benchmarks.

datapythonista · 2018-10-02T18:22:58Z

Thanks for the work on this @jerodestapa

TomAugspurger · 2018-10-02T18:39:35Z

For the pytest issue, pytest pandas should work.

I think we're doing something wrong with how we register new command line options, but I'm not familiar enough with pytest plugins to know how to fix it.

battlesdev · 2018-10-02T19:08:56Z

You bet, @datapythonista. Thanks for suggestions. I agree it's less than ideal, but as you say, it's the best solution given the constraints around flake8.

@TomAugspurger, it's not liking something about setup.cfg and the default option addopts = --strict-data-files --durations=10, but it may just be a local issue on my end. I'll let you know if I figure it out, though.

jreback · 2018-10-07T23:06:14Z

can you rebase

codecov · 2018-10-09T17:54:26Z

Codecov Report

Merging #22947 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #22947      +/-   ##
==========================================
- Coverage   92.19%   92.19%   -0.01%     
==========================================
  Files         169      169              
  Lines       50904    50911       +7     
==========================================
+ Hits        46933    46939       +6     
- Misses       3971     3972       +1

Flag	Coverage Δ
#multiple	`90.61% <ø> (-0.01%)`	⬇️
#single	`42.3% <ø> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/internals/concat.py	`98% <0%> (-0.38%)`	⬇️
pandas/core/dtypes/dtypes.py	`95.58% <0%> (+0.03%)`	⬆️

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 ca7d518...a04762d. Read the comment docs.

battlesdev · 2018-10-09T18:02:37Z

My apologies on the wait, @jreback. Let me know if anything else needs fixing.

Thanks.

datapythonista · 2018-10-09T18:06:04Z

Not sure if you've seen it @jerodestapa, but seems like you've got unrelated changes in this PR.

If you don't find a better way to fix it, usually the next steps could fix it (in the PR branch):

git fetch upstream
git merge upstream/master
git reset --soft upstream/master
git commit -m "your pr comment"
git push -f

This will delete the history of your changes, but should leave your changes and nothing else.

…s and added noqa: F401 to each

battlesdev · 2018-10-09T18:23:19Z

Thanks, @datapythonista. I must have done something weird when rebasing. I just reset and pushed again.

jreback · 2018-10-10T12:13:36Z

this looks fine. can you run the asv's just to be sure things still work.

battlesdev · 2018-10-10T16:47:32Z

@jreback: The asv's are still running, but it looks like the four files that updated when I rebased (indexing.py, join_merge.py, panel_ctor.py, and panel_methods.py) still have the from .pandas_vb_common import Panel line instead of importing Panel from pandas. Should I remove those lines and make another commit, or rebase again?

datapythonista · 2018-10-10T16:59:32Z

I think the PR is ok as it is. Not sure why Panel was being imported from pandas_vb_common.py instead of directly, but as your changes already fix the linting, and Panel will be removed soon anyway, I think it's worth just leaving it as it was.

I think this is ready to be merged, thanks for the work @jerodestapa

battlesdev · 2018-10-10T17:16:48Z

Awesome. Thanks for all the help, guys. This was my first PR for pandas, and it's just exciting to contribute. Thanks, again.

datapythonista · 2018-10-10T17:23:53Z

Hopefully not the last one @jerodestapa ;)

datapythonista added Code Style Code style, linting, code_checks Benchmark Performance (ASV) benchmarks labels Oct 2, 2018

datapythonista approved these changes Oct 2, 2018

View reviewed changes

pandas-dev#22885, Moved all setup imports to bottom of benchmark file…

a04762d

…s and added noqa: F401 to each

battlesdev force-pushed the setup-linting-fix branch from 7572888 to a04762d Compare October 9, 2018 18:18

jreback added this to the 0.24.0 milestone Oct 10, 2018

datapythonista merged commit 296c251 into pandas-dev:master Oct 10, 2018

battlesdev deleted the setup-linting-fix branch October 10, 2018 17:24

h-vetinari mentioned this pull request Oct 11, 2018

Fix ASV imports #23085

Merged

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

STYLE: Fixed linting of benchmarks (pandas-dev#22947)

a7f5d8e

simonjayhawkins mentioned this pull request Jan 2, 2019

Fix import formatting using isort #23334

Closed

gasparia405 mentioned this pull request Jan 27, 2019

CLN: isort asv_bench/benchmark/algorithms.py #24958

Merged

1 task

Uh oh!

Conversation

battlesdev commented Oct 2, 2018

Uh oh!

pep8speaks commented Oct 2, 2018

Uh oh!

datapythonista left a comment

Choose a reason for hiding this comment

Uh oh!

datapythonista commented Oct 2, 2018

Uh oh!

TomAugspurger commented Oct 2, 2018

Uh oh!

battlesdev commented Oct 2, 2018

Uh oh!

jreback commented Oct 7, 2018

Uh oh!

codecov bot commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

battlesdev commented Oct 9, 2018

Uh oh!

datapythonista commented Oct 9, 2018

Uh oh!

battlesdev commented Oct 9, 2018

Uh oh!

jreback commented Oct 10, 2018

Uh oh!

battlesdev commented Oct 10, 2018

Uh oh!

datapythonista commented Oct 10, 2018

Uh oh!

battlesdev commented Oct 10, 2018

Uh oh!

datapythonista commented Oct 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Oct 9, 2018 •

edited

Loading