Skip to content

WIP: verbose -> keyword-only arg in apply_baseline#8263

Closed
hoechenberger wants to merge 1 commit intomne-tools:masterfrom
hoechenberger:verbose
Closed

WIP: verbose -> keyword-only arg in apply_baseline#8263
hoechenberger wants to merge 1 commit intomne-tools:masterfrom
hoechenberger:verbose

Conversation

@hoechenberger
Copy link
Copy Markdown
Member

@hoechenberger hoechenberger commented Sep 15, 2020

Reference issue

Addresses part of #8258

@larsoner I didn't have much time tonight and this is how far I got. Feel free to bring this one home in time for 0.21; I probably won't have time to give it any more love before the weekend.

vars_ = vars(self)
if 'kwonlyargs' in vars_:
vars_['signature'] = vars_['signature'].replace('*, ', '')
src = src_templ % vars_
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general we try just to vendor this rather than make our own updates, so we should check to see if an updated version of https://github.com/micheles/decorator/blob/master/src/decorator.py will do what we want

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

So this is what happens when you don't check a file's entire path, and only look at its base name. 🤦‍♂️

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Which functionality from this package do we use that's not already in Python's standard library?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IIRC mostly the verbose decorator needs it to preserve traceback information properly. It's possible it's outdated for Python 3.6+, not sure

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've checked with the latest decorator release and the problem persists.

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