Skip to content

Conversation

bluetech
Copy link
Member

@bluetech bluetech commented Apr 24, 2020

[flake8 3.8.0 is not released yet, only an alpha -- marked WIP until it is released officially.]

The new flake8 (or more correctly the pyflakes version it uses) fixes several issues that come up a lot with type annotations, so I'm eager to update so I can stop typing all of those noqa's :)

bluetech added 3 commits May 12, 2020 09:29
New errors:

    testing/test_setupplan.py:104:15: E741 ambiguous variable name 'l'
    testing/test_setupplan.py:107:15: E741 ambiguous variable name 'l'
    extra/get_issues.py:48:29: E741 ambiguous variable name 'l'
    testing/test_error_diffs.py:270:32: E741 ambiguous variable name 'l'

Not so sure about it but easier to just fix.

But more importantly, is a large amount of typing-related issues there
were fixed which necessitated noqa's which can now be removed.
Mostly I wanted to remove uses of `noqa`.

In Python 3 the two are the same.
@bluetech bluetech changed the title WIP: pre-commit: update flake8 3.7.7 -> 3.8.0 pre-commit: update flake8 3.7.7 -> 3.8.1 May 12, 2020
@bluetech bluetech marked this pull request as ready for review May 12, 2020 06:36
@bluetech
Copy link
Member Author

I updated to the released flake8 version, so this is ready now.

try:
actual = np.asarray(actual)
except: # noqa
except BaseException:
Copy link
Member

Choose a reason for hiding this comment

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

as followup, i wonder if Exception would be a better limit

Copy link
Member Author

Choose a reason for hiding this comment

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

np.asarray seems to except any and all inputs, so I doubt it can even raise at all, but BaseException is definitely inappropriate here. I'll just go ahead and change it to Exception in this PR, I think it's safe to do.

try:
result = func()
except: # noqa
except BaseException:
Copy link
Member

Choose a reason for hiding this comment

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

if we catch it here, we can as a followup clean up the exceptioninfo usage, and bind a variable name + use isintstance

Copy link
Member Author

Choose a reason for hiding this comment

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

clean up the exceptioninfo usage

It would be be nice to have a ExceptionInfo.from_exception(e) which does ExceptionInfo.from_exc_info((type(e), e, e.__traceback__)), but since it doesn't exist yet the current code seems a bit cleaner.

Copy link
Member

Choose a reason for hiding this comment

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

all we use it for is a reraise, thats why i think we do not need the actual object at all

except ... as e:
  if reraise is not None and isinstance(e, reraise):
    raise

Copy link
Member

Choose a reason for hiding this comment

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

oh eh im wrong, its necessary just below

however on pytohn3 we should deprecate from_current, and add a from_exception method, as that old style 3-tuple is no longer sensible and necessary

definitively something for a followup

i want to see a complete replacement for exceptioninfo mit to long term

I'm not sure if it can even raise at all, but catching BaseException
would swallow ctrl-C and such and is definitely inappropriate here.
@bluetech bluetech merged commit 9527622 into pytest-dev:master May 12, 2020
@bluetech bluetech deleted the update-flake8 branch June 17, 2020 15:00
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.

2 participants