Today was my second day of
Enhackathon, and I spent most of it writing docstrings, while poking a few other threads on BPO.
What I did
Failing test case
Issue 36092, regarding a failing test case,
test_patch_descriptor, had been opened in February. Using the handy
git log -S "<search string>" command, I was able to establish that the testcase in question had been removed in May in the course of some related changes, and get the issue closed. Not exactly a large contribution but one fewer active issue, and all before coffee!
I spent over half of the day updating/adding class-level docstrings - sounds slow but there are a lot of classes! Doing this involved doing some research about what certain classes and methods were used for, so was educational for me as well as hopefully being useful for others in the future.
I’ve uploaded my diff as it is and am waiting to get comments/markups from the dev on the BPO thread. Fingers crossed it’ll be ready for a PR after a few tweaks.
MagicMock and divmod
MagicMock is an incredibly useful way of mocking out classes/functions when testing your code, because it’s very easy to set-up and use. You can do pretty much anything with
MagicMocks and they’ll just give you more
MagicMock instances, like some terrifying calculus problem. However, the way that people tend to use
divmod conflicts with that.
>>> from unittest.mock import * >>> floordiv, mod = divmod(5, 2) >>> floordiv 2 >>> mod 1 >>> floordiv, mod = divmod(MagicMock(), 2) Traceback (most recent call last): File "<stdin>", line 1, in <module> ValueError: not enough values to unpack (expected 2, got 0) >>>
This isn’t a problem with
MagicMock per-se, but rather with the way that people tend to unpack the result of divmod, assigning the integer division and modulus result straight away. However, we’re used to MagicMock being able to handle anything thrown at it, so there are a couple of solutions (first three suggested on the BPO thread).
Return some constant value when
MagicMock.__divmod__gets called, e.g.
(1,0). This could then be happily unpacked. Not very flexible though.
Return a pair of
MagicMockinstances. This is very sensible and in-keeping with the rest of the behaviour of
Redefine the default behaviour of
MagicMock.__divmod__in terms of
MagicMock.__mod__. This would return the same as (2) by default, but if the user were to set return values for
__mod__, the result of the
divmodcall would be changed appropriately. I think this is pretty tasty but it’s a fairly niche case and might not fit with how
MagicMockusually behaves. Would definitely have to be documented!
Change the behaviour of
MagicMockmore generally such that trying to unpack a
MagicMockinstance into two variables, for example, would assign a new
MagicMockinstance to each. This would be a bit more far-reaching than this issue but might be worth thinking about. This would fix the issue as a side-effect, as the current result of e.g.
divmod(MagicMock(), 2)is another instance of
I had initially thought (3) was best but have been mulling it over and now prefer one of (2) or (4). Time for discussion on the BPO thread!
I enjoyed my full day of contribution - the first PR remains elusive but I think I have made a decent start. Doc-writing was surprisingly difficult, summing up some of the ABCs in a way that was non-trivial and helpful was not always easy. I’m looking forward to being able to argue the case for (2) or (4) and hopefully make a start on implementing one of them next time.