STYLE: Specify bare exceptions in pandas/tests #23370

alexander-ponomaroff · 2018-10-27T02:32:16Z

closes Replace bare excepts by explicit excepts in pandas/tests/ #22872
passes git diff upstream/master -u -- "*.py" | flake8 --diff

The empty except statements now capture specific exceptions.

Following warnings are fixed:

pandas/tests/test_panel.py:338:13: E722 do not use bare 'except'
pandas/tests/test_panel.py:344:13: E722 do not use bare 'except'
pandas/tests/test_nanops.py:144:9: E722 do not use bare 'except'
pandas/tests/test_nanops.py:149:9: E722 do not use bare 'except'
pandas/tests/test_nanops.py:170:21: E722 do not use bare 'except'
pandas/tests/test_nanops.py:174:21: E722 do not use bare 'except'
pandas/tests/test_multilevel.py:1381:9: E722 do not use bare 'except'
pandas/tests/test_strings.py:2632:13: E722 do not use bare 'except'
pandas/tests/io/test_pytables.py:54:9: E722 do not use bare 'except'
pandas/tests/io/test_pytables.py:62:5: E722 do not use bare 'except'
pandas/tests/io/test_pytables.py:120:5: E722 do not use bare 'except'
pandas/tests/io/test_pytables.py:4624:21: E722 do not use bare 'except'
pandas/tests/io/test_sql.py:1793:9: E722 do not use bare 'except'
pandas/tests/io/test_sql.py:2378:9: E722 do not use bare 'except'
pandas/tests/io/test_sql.py:2405:9: E722 do not use bare 'except'
pandas/tests/io/formats/test_format.py:73:5: E722 do not use bare 'except'
pandas/tests/io/formats/test_format.py:455:13: E722 do not use bare 'except'
pandas/tests/indexing/common.py:154:13: E722 do not use bare 'except'
pandas/tests/indexing/common.py:217:17: E722 do not use bare 'except'

pep8speaks · 2018-10-27T02:32:31Z

Hello @alexander-ponomaroff! Thanks for updating the PR.

There are no PEP8 issues in the file doc/source/conf.py !
There are no PEP8 issues in the file pandas/tests/indexing/common.py !
There are no PEP8 issues in the file pandas/tests/io/formats/test_format.py !
There are no PEP8 issues in the file pandas/tests/io/test_pytables.py !
There are no PEP8 issues in the file pandas/tests/io/test_sql.py !
There are no PEP8 issues in the file pandas/tests/test_multilevel.py !
There are no PEP8 issues in the file pandas/tests/test_nanops.py !
There are no PEP8 issues in the file pandas/tests/test_panel.py !
There are no PEP8 issues in the file pandas/tests/test_strings.py !

Comment last updated on November 13, 2018 at 04:43 Hours UTC

datapythonista

lgtm, thanks!

datapythonista · 2018-10-27T03:47:11Z

I think this is the last chunk of bare exceptions. Can you run flake8 --select=E722 --config=none . in your branch, and if nothing is being reported, edit setup.cfg and pep8speaks.yml to stop ignoring E722 please?

alexander-ponomaroff · 2018-10-27T03:54:51Z

@datapythonista

When running: flake8 --select=E722 --config=none pandas/tests/
Nothing is reported

When running: flake8 --select=E722 --config=none
./doc/source/conf.py:568:9: E722 do not use bare 'except'
./doc/source/conf.py:573:5: E722 do not use bare 'except'
./doc/source/conf.py:580:5: E722 do not use bare 'except'

Should I fix those also?

datapythonista · 2018-10-27T03:57:22Z

Yes, that would be great. This will finish all them and we can avoid new bare excepts using the CI from now on with the changes I mentioned.

alexander-ponomaroff · 2018-10-27T07:25:08Z

@datapythonista I had some trouble with pandas/tests/indexing/common.py - line 217. Could not figure out which exceptions needed to go there. My previous commit looks like passed the first CI, so it's looking promising, although I have 4 different exceptions on line 217.

I put the final exceptions in doc/source and removed E722 from the ignore lists. Hopefully it passes all the CI tests.

If you have some advice about line 217 of pandas/tests/indexing/common.py, it would be much appreciated. That's the only one that gave me some trouble and I don't think all 4 of those exceptions are correct. I couldn't get pytest to run, so had to wait for CI to perform testing.

datapythonista · 2018-10-27T09:15:38Z

Can you remove E722 from the ignores in pep8speaks.yml too please?

I don't have access to a computer at the moment, @jreback, can you check if the exceptions in common.py L217 make sense to you?

alexander-ponomaroff · 2018-10-27T16:50:06Z

@datapythonista Weird, I removed it from pep8speaks.yml but it didn't get added to the commit.

https://travis-ci.org/pandas-dev/pandas/jobs/447018587

I am unsure why I fail this check, it doesn't show any errors except this:
The command "ci/code_checks.sh" exited with 1.

Also, it looks like that line 217 of common.py is still not working properly as it fails one of the tests of Travis CI, so I will put except: Exception on that line for now.
https://travis-ci.org/pandas-dev/pandas/jobs/447018588

Other than these two tests, everything else passes.

jreback · 2018-10-31T13:05:24Z

can you rebase

datapythonista · 2018-11-02T16:40:33Z

@alexander-ponomaroff the error in travis looks like a genuine error introduced in your changes, I think pymysql needs to be imported. Can you take a look and leave the CI green so we can merge. Thanks!

datapythonista · 2018-11-04T17:20:50Z

@alexander-ponomaroff do you have time to fix the CI problem?

alexander-ponomaroff · 2018-11-04T18:32:04Z

Yes, I will work on this tonight. I was away from a computer on Friday and Saturday.

…

On Nov 4, 2018, at 12:21 PM, Marc Garcia ***@***.***> wrote: @alexander-ponomaroff do you have time to fix the CI problem? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

alexander-ponomaroff · 2018-11-05T05:57:43Z

@datapythonista My checks passed after updated pymysql. I had to resolve a merge conflict after, so hopefully nothing messes up and the checks pass again.

datapythonista

Can you also rebase and fix the conflicts please? Thanks!

pandas/tests/io/test_sql.py

datapythonista · 2018-11-05T06:02:27Z

I think the linting ci/code_checks.py lint will fail with a pep8 error because of the import, as I mentioned. Let's see if we can leave it in green, and I'll try to get it merged asap, so you don't have to fix more conflicts.

datapythonista · 2018-11-08T16:45:51Z

pandas/conftest.py

 import numpy as np
 import pytest

 from pandas.compat import PY3
 import pandas.util._test_decorators as td

+import hypothesis
+from hypothesis import strategies as st


Not very important, but I don't understand this change. The standard is to import Python stdlib first, third-party packages (like hypothesis) later, and the project code (pandas) last.

isort did that, I ran it on the whole repository

So whenever somebody else runs it and since pandas/conftest.py is not in setup.cfg, they should get this change also with isort. Unless something went wrong.

That's weird. Not so important as I said, but if you have time, I'd remove this file from the PR, as nothing really changed here more than the isort changes, which are unrelated. Or if you want to keep, may be we should remove this file from the excludes of isort (assuming it's there).

@alimcmaster1 can you take a look here? May be I'm missing something?

Looks like the CI agrees with me:

ERROR: /home/travis/build/pandas-dev/pandas/pandas/conftest.py Imports are incorrectly sorted. ERROR: /home/travis/build/pandas-dev/pandas/pandas/tests/io/test_sql.py Imports are incorrectly sorted.

It's probably a bug in isort when sorting the files, validation seems right.

Can you fix, so the CI is green and we can merge?

Thanks @datapythonista will take a look at what is going on with isort and that file this evening.

I reverted everything that isort did and added test_sql.py back to setup.cfg. I tried running isort a few times, but it just sorts everything the way it sorted the first time.

Running isort conftest.py ran OK for me on master. It should already be sorted fine as its not in the setup.cfg. What version of isort are you using? Im running 4.3.4

@alimcmaster1 My version is 4.3.4 as well.

pandas/tests/io/test_sql.py

codecov · 2018-11-10T22:06:43Z

Codecov Report

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

@@           Coverage Diff           @@
##           master   #23370   +/-   ##
=======================================
  Coverage   92.25%   92.25%           
=======================================
  Files         161      161           
  Lines       51237    51237           
=======================================
  Hits        47269    47269           
  Misses       3968     3968

Flag	Coverage Δ
#multiple	`90.63% <ø> (ø)`	⬆️
#single	`42.29% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/indexes/base.py	`96.45% <0%> (ø)`	⬆️

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 adc54fe...c55692f. Read the comment docs.

codecov · 2018-11-10T22:06:43Z

Codecov Report

❗ No coverage uploaded for pull request base (master@960a73f). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #23370   +/-   ##
=========================================
  Coverage          ?   92.24%           
=========================================
  Files             ?      161           
  Lines             ?    51433           
  Branches          ?        0           
=========================================
  Hits              ?    47446           
  Misses            ?     3987           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.64% <ø> (?)`
#single	`42.28% <ø> (?)`

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 960a73f...6a86779. Read the comment docs.

datapythonista · 2018-11-10T22:36:44Z

lgtm, let's see if it finishes on green

jreback · 2018-11-11T18:41:47Z

looks like still a linting error:

Linting .py code
./pandas/tests/io/test_sql.py:1819:1: W293 blank line contains whitespace

jreback · 2018-11-11T18:42:49Z

@datapythonista does this fully close #22872 . ?

datapythonista · 2018-11-11T18:53:07Z

@datapythonista does this fully close #22872 . ?

Yes, this is actually the last chunk of bare except fixes. We remove the ignore of the flake8 error here, so we start validating that no more are added in this PR.

alexander-ponomaroff · 2018-11-13T14:18:21Z

@datapythonista All checks pass now.

alimcmaster1

LGTM - CC @datapythonista and @jreback to merge

datapythonista · 2018-11-13T21:31:41Z

Looks great. Thanks for the fixes @alexander-ponomaroff

@jreback, can you take a look and merge if you're happy?

jreback · 2018-11-18T18:38:13Z

lgtm. just merged master to be sure. let's merge on green.

jreback · 2018-11-18T22:28:57Z

rebased. can merge on green.

jreback · 2018-11-19T01:25:31Z

thanks @alexander-ponomaroff

alexander-ponomaroff · 2018-11-19T05:34:12Z

@jreback @datapythonista No problem guys :)

…fixed * upstream/master: (46 commits) DEPS: bump xlrd min version to 1.0.0 (pandas-dev#23774) BUG: Don't warn if default conflicts with dialect (pandas-dev#23775) BUG: Fixing memory leaks in read_csv (pandas-dev#23072) TST: Extend datetime64 arith tests to array classes, fix several broken cases (pandas-dev#23771) STYLE: Specify bare exceptions in pandas/tests (pandas-dev#23370) ENH: between_time, at_time accept axis parameter (pandas-dev#21799) PERF: Use is_utc check to improve performance of dateutil UTC in DatetimeIndex methods (pandas-dev#23772) CLN: io/formats/html.py: refactor (pandas-dev#22726) API: Make Categorical.searchsorted returns a scalar when supplied a scalar (pandas-dev#23466) TST: Add test case for GH14080 for overflow exception (pandas-dev#23762) BUG: Don't extract header names if none specified (pandas-dev#23703) BUG: Index.str.partition not nan-safe (pandas-dev#23558) (pandas-dev#23618) DEPR: tz_convert in the Timestamp constructor (pandas-dev#23621) PERF: Datetime/Timestamp.normalize for timezone naive datetimes (pandas-dev#23634) TST: Use new arithmetic fixtures, parametrize many more tests (pandas-dev#23757) REF/TST: Add more pytest idiom to parsers tests (pandas-dev#23761) DOC: Add ignore-deprecate argument to validate_docstrings.py (pandas-dev#23650) ENH: update pandas-gbq to 0.8.0, adds credentials arg (pandas-dev#23662) DOC: Improve error message to show correct order (pandas-dev#23652) ENH: Improve error message for empty object array (pandas-dev#23718) ...

Fix issue pandas-dev#22872 - Capture specific exceptions

cd65d8e

Some fixes

066cdcf

datapythonista approved these changes Oct 27, 2018

View reviewed changes

alexander-ponomaroff added 5 commits October 27, 2018 00:20

Further fixes

318dd54

Test if the problem is in common.py line 217

677b707

Issue confirmed line 217 of common.py, testing exceptions

fa0b03b

Working through exceptions

2f57297

Working through exceptions

6cca7d7

gfyoung added Code Style Code style, linting, code_checks Clean labels Oct 27, 2018

Fix final bare excepts and stop ignoring E722

6967323

Added assertion error to line 217 of common.py

91c8af2

Line 217 still problematic

3fc4499

Rebase

12b8c34

alexander-ponomaroff and others added 2 commits November 4, 2018 23:13

Update pymysql errors

f055a09

Merge branch 'master' into issue-22872

4f08510

datapythonista reviewed Nov 5, 2018

View reviewed changes

pandas/tests/io/test_sql.py Outdated Show resolved Hide resolved

alexander-ponomaroff added 4 commits November 8, 2018 11:33

Rebase

85e1c9c

Merge conflict fix

74ce78c

isort Run on pandas, test_sql.py removed from setup.cfg exclusion list

39944b6

merge

c276bd3

datapythonista reviewed Nov 8, 2018

View reviewed changes

Reverted isort changes

48ad6f0

datapythonista reviewed Nov 8, 2018

View reviewed changes

pandas/tests/io/test_sql.py Show resolved Hide resolved

datapythonista mentioned this pull request Nov 9, 2018

Remove bare exceptions from 'pandas/tests' to decrease flake8 E722 warnings #22882

Closed

4 tasks

import pymysql in the beginning of function

c55692f

jreback added this to the 0.24.0 milestone Nov 11, 2018

Remove empty line 1819

ff36b21

alimcmaster1 approved these changes Nov 13, 2018

View reviewed changes

Merge branch 'master' into PR_TOOL_MERGE_PR_23370

bd3e656

Merge branch 'master' into PR_TOOL_MERGE_PR_23370

6a86779

jreback merged commit e538182 into pandas-dev:master Nov 19, 2018

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

STYLE: Specify bare exceptions in pandas/tests (pandas-dev#23370)

f3cd65a

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

STYLE: Specify bare exceptions in pandas/tests (pandas-dev#23370)

6f23d5a

STYLE: Specify bare exceptions in pandas/tests #23370

STYLE: Specify bare exceptions in pandas/tests #23370

Conversation

alexander-ponomaroff commented Oct 27, 2018 • edited by datapythonista Loading

pep8speaks commented Oct 27, 2018 • edited Loading

Comment last updated on November 13, 2018 at 04:43 Hours UTC

datapythonista left a comment

Choose a reason for hiding this comment

datapythonista commented Oct 27, 2018

alexander-ponomaroff commented Oct 27, 2018

datapythonista commented Oct 27, 2018

alexander-ponomaroff commented Oct 27, 2018

datapythonista commented Oct 27, 2018 • edited Loading

alexander-ponomaroff commented Oct 27, 2018

jreback commented Oct 31, 2018

datapythonista commented Nov 2, 2018

datapythonista commented Nov 4, 2018

alexander-ponomaroff commented Nov 4, 2018 via email

alexander-ponomaroff commented Nov 5, 2018

datapythonista left a comment

Choose a reason for hiding this comment

datapythonista commented Nov 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alimcmaster1 Nov 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 10, 2018

Codecov Report

codecov bot commented Nov 10, 2018 • edited Loading

Codecov Report

datapythonista commented Nov 10, 2018

jreback commented Nov 11, 2018

jreback commented Nov 11, 2018

datapythonista commented Nov 11, 2018

alexander-ponomaroff commented Nov 13, 2018

alimcmaster1 left a comment

Choose a reason for hiding this comment

datapythonista commented Nov 13, 2018

jreback commented Nov 18, 2018

jreback commented Nov 18, 2018

jreback commented Nov 19, 2018

alexander-ponomaroff commented Nov 19, 2018

alexander-ponomaroff commented Oct 27, 2018 •

edited by datapythonista

Loading

pep8speaks commented Oct 27, 2018 •

edited

Loading

datapythonista commented Oct 27, 2018 •

edited

Loading

alimcmaster1 Nov 8, 2018 •

edited

Loading

codecov bot commented Nov 10, 2018 •

edited

Loading