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

STYLE: Fix lint error "F811 redefinition of unused 'setup'" in benchmarks #22885

Closed
datapythonista opened this issue Sep 29, 2018 · 4 comments · Fixed by #22947
Closed

STYLE: Fix lint error "F811 redefinition of unused 'setup'" in benchmarks #22885

datapythonista opened this issue Sep 29, 2018 · 4 comments · Fixed by #22947
Labels
CI Continuous Integration Code Style Code style, linting, code_checks good first issue Needs Discussion Requires discussion from core team before further action
Milestone

Comments

@datapythonista
Copy link
Member

(related to #22884)

There is an issue in the linting of the benchmarks, caused by how asv manages reusing a setup function. See:

(pandas-dev) [mgarcia@xps asv_bench]$ flake8 --config=none benchmarks/categoricals.py 
benchmarks/categoricals.py:21:5: F811 redefinition of unused 'setup' from line 14

The problem is that to make asv reuse a common setup function for all the benchmarks in a file, it is expected to have it defined in the top-level namespace. Then every benchmark class in the file can specify a setup method that will be executed after the common one. See this sample code:

from .pandas_vb_common import setup

class MyBenchmark:
    def setup(self):
        pass

Regardless if this is a good approach by asv, the problem is that the previous code causes linting problems (the error shown before). In this case, the setup defined in pandas_vb_common.py simply defines the seed, np.random.seed(1234).

It's not a great option, but we could move the import of setup at the end of the file (so, the methods are not seen as redefinitions), and flag it to ignore the validation of not being used: from .pandas_vb_common import setup # noqa: F401. But I find this one trickier, as it's difficult to see that there is a common setup being executed, unless you go at the end of the file.

Those are the affected files:

benchmarks/ctors.py
benchmarks/series_methods.py
benchmarks/panel_methods.py
benchmarks/inference.py
benchmarks/replace.py
benchmarks/groupby.py
benchmarks/algorithms.py
benchmarks/timeseries.py
benchmarks/binary_ops.py
benchmarks/stat_ops.py
benchmarks/index_object.py
benchmarks/frame_methods.py
benchmarks/panel_ctor.py
benchmarks/eval.py
benchmarks/reshape.py
benchmarks/categoricals.py
benchmarks/rolling.py
benchmarks/gil.py
benchmarks/indexing.py
benchmarks/multiindex_object.py
benchmarks/sparse.py
benchmarks/frame_ctor.py
benchmarks/plotting.py
benchmarks/join_merge.py
benchmarks/attrs_caching.py
benchmarks/reindex.py
benchmarks/io/stata.py
benchmarks/io/csv.py
benchmarks/io/sql.py
benchmarks/io/json.py
benchmarks/io/msgpack.py
benchmarks/io/hdf.py
benchmarks/io/excel.py
benchmarks/io/pickle.py
@datapythonista datapythonista added CI Continuous Integration Code Style Code style, linting, code_checks Effort Low Needs Discussion Requires discussion from core team before further action good first issue labels Sep 29, 2018
@battlesdev
Copy link
Contributor

Thanks, Marc. I'll give this one a go.

@jorisvandenbossche
Copy link
Member

@datapythonista isn't that an error in flake8? You are perfectly allowed to have a function and a method with the same name no? (it's not a "redefinition", they don't overwrite each other?)

@datapythonista
Copy link
Member Author

I thought the same, but unfortunately that's not what flake8 things... :\

And even if the methods had other names, the error would be that setup is imported but not used. In that case it'd be as easy as adding a noqa. But with the methods with the same name, the error is not in the import, but in every method definition.

@jorisvandenbossche
Copy link
Member

I thought the same, but unfortunately that's not what flake8 things... :\

Yeah, an issue about it is here: PyCQA/pyflakes#268

battlesdev added a commit to battlesdev/pandas that referenced this issue Oct 9, 2018
@jreback jreback added this to the 0.24.0 milestone Oct 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Code Style Code style, linting, code_checks good first issue Needs Discussion Requires discussion from core team before further action
Projects
None yet
4 participants