Skip to content

remove redundant unittest.mock imports#203

Merged
nicoddemus merged 1 commit intopytest-dev:masterfrom
graingert:remove-redundant-code
Aug 29, 2020
Merged

remove redundant unittest.mock imports#203
nicoddemus merged 1 commit intopytest-dev:masterfrom
graingert:remove-redundant-code

Conversation

@graingert
Copy link
Copy Markdown
Member

No description provided.

Comment on lines -90 to +87
:rtype: mock.MagicMock
:rtype: unittest.mock.MagicMock
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.

do we care about :rtype:s here? can we use sphinx-autodoc-typehints instead?

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.

Hey @graingert!

IMHO we can drop all types from the docs and rely solely on the type annotations.

About sphinx, we don't really have sphinx docs at the moment, so I don't think it matters.

Want to tackle this in this PR?

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 think worth defering to another pr

@graingert graingert requested a review from nicoddemus August 24, 2020 14:49
@nicoddemus
Copy link
Copy Markdown
Member

I don't think this applies anymore given we use unittest.mock.MagicMock for annotations.

Feel free to reopen if I'm wrong however!

@nicoddemus nicoddemus closed this Aug 29, 2020
@graingert graingert reopened this Aug 29, 2020
@graingert
Copy link
Copy Markdown
Member Author

unittest.mock is imported 3 times, we only need it once

@nicoddemus nicoddemus closed this Aug 29, 2020
@nicoddemus nicoddemus reopened this Aug 29, 2020
@nicoddemus
Copy link
Copy Markdown
Member

Ahh right, my bad! Sorry about that.

@nicoddemus nicoddemus merged commit 632af5e into pytest-dev:master Aug 29, 2020
@graingert graingert deleted the remove-redundant-code branch August 29, 2020 11:06
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