bpo-27200: fix configparser, copyreg and ctypes doctests by marco-buttu · Pull Request #240 · python/cpython

@marco-buttu

This PR patially fixes bpo-27200. Partially and not completely, because I followed the suggestion of @ezio-melotti to split the patch in several patches. According to @rhettinger I applyed minimal changes, skipping some tests instead of using sorted(). I just want to point out that the two <BLANKLINE> and the testsetup dicrective will not appear in the output.

@marco-buttu

berkerpeksag


.. testsetup::

import configparser

Choose a reason for hiding this comment

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

Indentation should use three spaces here.

Choose a reason for hiding this comment

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

Thanks, you are right

accessing it repeatedly returns the same object each time. On the other hand,
accessing it through an index returns a new object each time:

.. doctest::

Choose a reason for hiding this comment

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

I find your version much clearer, +1. However, I don't see much point converting this to a doctest. I'd say remove .. doctest:: and keep the rest of the changes.

Choose a reason for hiding this comment

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

I removed the doctest directive. However, there is one point for taking it: in some editors, if you use the :: before the example you enable the syntax hightligh for the example, but the example will not be doctested. If you use just the :, you lose the syntax highligh but the example will be doctested. If you eventually use the doctest directive, you have both syntax highlight and doctest.

Choose a reason for hiding this comment

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

Well, I didn't mentioned it because I assumed you are going to change "[...] each time:" to "[...] each time::" :)

Choose a reason for hiding this comment

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

@berkerpeksag, it was "[...] each time:" from the beginning, I did not change it. Do you still want to change it anyway? Sorry for all these questions :/

Choose a reason for hiding this comment

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

Yes, please change it to "[...] each time::".

Choose a reason for hiding this comment

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

Ok, done. Thank you! :-)

@marco-buttu

@marco-buttu

ezio-melotti

berkerpeksag

This was referenced

Mar 10, 2017

jaraco pushed a commit that referenced this pull request

Dec 2, 2022