Skip to content

Do not require doctests for dunder methods - #39

Closed
yairfrid wants to merge 1 commit into
TheAlgorithms:masterfrom
yairfrid:dont_require_doctest_for_dunders
Closed

yairfrid wants to merge 1 commit into
TheAlgorithms:masterfrom
yairfrid:dont_require_doctest_for_dunders

Conversation

@yairfrid

Copy link
Copy Markdown

nothing much to add, part of TheAlgorithms/Python#4352

@yairfrid

Copy link
Copy Markdown
Author

i also ran my formatter, please let me know if i should revert it

@dhruvmanila dhruvmanila left a comment

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.

Thanks for doing this. I completely forgot that we had a reason to not skip those dunder methods, but it makes sense to skip them. What do you say? @cclauss

Also, what formatter did you run? It changed a few things in a not so good way lol. We use pre-commit so if you enable it, you can just forget about the formatting part ;)

@cclauss

cclauss commented Apr 30, 2021

Copy link
Copy Markdown
Member

My sense is that some dunder methods should be tested like __iter__(), __repr__(), __str__() because users of the class will depend on them. So instead of a blanket statement that no dunder methods should be tested, how about an ignore list of dunder methods that do not need to be tested.

@yairfrid

Copy link
Copy Markdown
Author

Makes sense.
For that reason I'm going to ignore the following:
init (already ignored currently)
all equality and order operators: eq, le, lt, ge, gt
As those are the ones that were mentioned on my PR
I will also revert the formatting
Are there any more dunders you would add right now or do you think we should keep it as lean as we need it?

@yairfrid

Copy link
Copy Markdown
Author

BTW the formatter is default Pycharm

@yairfrid

Copy link
Copy Markdown
Author

Or actually, when rethinking it, the equality methods maybe should also have docstrings
I'll narrow down the problem we had, the methods were abstract, and abstract dunder methods may not need to be tested
Maybe we need to purify this requirement a bit more.

@cclauss

cclauss commented Apr 30, 2021 •

Copy link
Copy Markdown
Member

PyCharm does not come from the Python Software Foundation but psf/Black does.

@yairfrid yairfrid closed this May 9, 2021
@yairfrid

yairfrid commented May 9, 2021

Copy link
Copy Markdown
Author

Decided to close, this solution needs more thinking

@yairfrid
yairfrid deleted the dont_require_doctest_for_dunders branch May 9, 2021 10:37
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.

4 participants