Vector2.__new__ violates its interface contract

[nbraud]

While working on #144, I discovered that instances of Vector2 cannot be copied (using copy.{copy,deepcopy}), and I believe this is because we are violating the interface contract for __new__

• Provide a test reproducing the issue.
• Fix the issue.
• Update Update expectations of subclasses #144 so we do not have broken examples and expectations in the documentation.

[nbraud]

@astronouth7303 Assigning this to you because I do not know well-enough the internals of the Python object model :3

[nbraud]

According to __new__'s spec. __init__ is called automatically afterwards, so I moved more of the logic back in it (with the added benefit that subclasses will get the non-copying behaviour “for free”).
It looks like this resulted in some progress:

>>> import copy
>>> copy.copy(Vector2(0, 0))                                                                 
---------------------------------------------------------------------------
FrozenInstanceError                       Traceback (most recent call last)
<ipython-input-6-7fbabe237cc4> in <module>
----> 1 copy.copy(Vector2(0, 0))

/usr/lib/python3.7/copy.py in copy(x)
    104     if isinstance(rv, str):
    105         return x
--> 106     return _reconstruct(x, None, *rv)
    107 
    108 

/usr/lib/python3.7/copy.py in _reconstruct(x, memo, func, args, state, listiter, dictiter, deepcopy)
    290             if slotstate is not None:
    291                 for key, value in slotstate.items():
--> 292                     setattr(y, key, value)
    293 
    294     if listiter is not None:

<string> in __setattr__(self, name, value)

FrozenInstanceError: cannot assign to field 'x'

Clearly something is still broken, though. I checked, and regular dataclasses are copyable without issue:

>>> from dataclasses import dataclass                                                        

>>> @dataclass(frozen=True) 
>>> class Foo: 
>>>     bar: str 
>>>     weight: float 

>>> copy.copy(Foo("blah", 0.5))                                                              
Foo(bar='blah', weight=0.5)

[nbraud]

pings @astronouth7303

I ended up merging this branch into #144, because I ended up running into some of the issues fixed here, but I'd prefer to get this reviewed and merged first.

The specific issue with pickle and copy was fixed when you implemented __reduce__, but I think protocol violations are worth fixing (especially since this one manifested itself as hard-to-debug wonkyness).

[nbraud]

Rebased to deal with a merge conflict.

[AstraLuma]

I'm just not buying that an argumentless __new__ is part of the contract.

[nbraud]

How come that pickle (and possibly other modules) make that assumption, then?

I guess I should split this into several PRs, and only keep the controversial __new__/__init__ change here. (Update: done, 664a287 is all that is left in this branch)

[nbraud]

PS: Since I extracted the fix for __reduce__ to #157, this fails in hilarious ways.
I will pull #157 into this branch, but please merge it first.

[AstraLuma]

It appears that when a custom __reduce__/__reduce_ex__/etc (there's like 5 possible dunder methods?) is not defined, pickle defaults to saving the __dict__, calling argless __new__, and restoring the __dict__.

This is fine for many standard Python classes (eg, the engine, systems, scenes, and sprites), this assumption falls apart for a slots-based immutable class.

Keep in mind that pickle is standard library, not core interpreter. It's doing a lot of magic, but is making some assumptions while doing so.

[AstraLuma]

Going to close this.