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

Refactor out coarsen tests #5474

Merged
merged 4 commits into from
Jun 21, 2021
Merged

Conversation

dcherian
Copy link
Contributor

@dcherian dcherian commented Jun 16, 2021

Some questions:

  1. flake8 fails with some false positives. What do we do about that?
  2. I am importing the da and ds fitures from test_dataarray and test_dataset. Is that the pattern we want to follow?
xarray/tests/test_coarsen.py:9:1: F401 '.test_dataarray.da' imported but unused
xarray/tests/test_coarsen.py:10:1: F401 '.test_dataset.ds' imported but unused
xarray/tests/test_coarsen.py:13:36: F811 redefinition of unused 'ds' from line 10
xarray/tests/test_coarsen.py:20:26: F811 redefinition of unused 'ds' from line 10
xarray/tests/test_coarsen.py:35:25: F811 redefinition of unused 'ds' from line 10
xarray/tests/test_coarsen.py:51:5: F811 redefinition of unused 'da' from line 9
xarray/tests/test_coarsen.py:62:5: F811 redefinition of unused 'da' from line 9
xarray/tests/test_coarsen.py:84:5: F811 redefinition of unused 'ds' from line 10
xarray/tests/test_coarsen.py:156:5: F811 redefinition of unused 'ds' from line 10
xarray/tests/test_coarsen.py:186:25: F811 redefinition of unused 'ds' from line 10
xarray/tests/test_coarsen.py:217:5: F811 redefinition of unused 'da' from line 9
xarray/tests/test_coarsen.py:269:5: F811 redefinition of unused 'da' from line 9

@dcherian dcherian requested a review from max-sixty June 16, 2021 15:52
@pep8speaks
Copy link

pep8speaks commented Jun 16, 2021

Hello @dcherian! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 9:1: F401 '.test_dataarray.da' imported but unused
Line 10:1: F401 '.test_dataset.ds' imported but unused
Line 13:36: F811 redefinition of unused 'ds' from line 10
Line 20:26: F811 redefinition of unused 'ds' from line 10
Line 35:25: F811 redefinition of unused 'ds' from line 10
Line 51:5: F811 redefinition of unused 'da' from line 9
Line 62:5: F811 redefinition of unused 'da' from line 9
Line 84:5: F811 redefinition of unused 'ds' from line 10
Line 156:5: F811 redefinition of unused 'ds' from line 10
Line 186:25: F811 redefinition of unused 'ds' from line 10
Line 217:5: F811 redefinition of unused 'da' from line 9
Line 269:5: F811 redefinition of unused 'da' from line 9
Line 291:28: F811 redefinition of unused 'da' from line 9

Comment last updated at 2021-06-16 16:40:59 UTC

@max-sixty
Copy link
Collaborator

Great!! We can import the fixtures, or we can promote them to conftest.py and pytest will automatically use them.

I had the same thing with flake8 IIRC; I think we can just noqa the imports.

setup.cfg Outdated Show resolved Hide resolved
@dcherian dcherian mentioned this pull request Jun 16, 2021
5 tasks
@dcherian dcherian merged commit 5381962 into pydata:master Jun 21, 2021
@dcherian dcherian deleted the refactor-coarsen-tests branch June 21, 2021 16:35
@github-actions
Copy link
Contributor

Unit Test Results

0 files  ±0  0 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 5381962. ± Comparison against base commit 5381962.

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