Issue1682423

Here's a patch against the 2.3 branch converting PyModule.java to a new-style class.

I've also added Lib/tests/test_module.py, from CPython, with some extra added tests

I've changed test_descr.py and the newly added test_module.py to reference ModuleType from types.ModuleType instead of type(sys). I've also commented out a small test in test_module.py that tests MoudleType.__doc__ (I think this might be a PyType or PyObject issue)

PyModule is a little tricky, a couple things I'd point out are:

o __findattr__ is like the old __findattr__, except it now works with an empty __dict__ (__dict__ can be null (None) after ModuleType.__new__())

o __setattr__ has to do some special handling for when __dict__ == null. It might pay to just hardcode the __dict__ access (which simply raises a TypeError) upfront in __set/get/delattr__ instead of having a __dict__ PyGetSetDescr

I haven't tried making an actual java subclass of PyModule yet (e.g. [1639028]). I took a brief look at doing it for PySystemState, but that seemed like it required more than a few changes (doing that would make type(sys) == types.ModuleType)

I'm not sure if the 2.2 branch needs this

Thanks for the patch, and for taking care to include updated test cases. The __doc__ problem may be the one reported in bug 1605006, so i think you're correct not to worry about it here.

Minor thing: i have the vague notion that the null __dict__ behavior may be cpython implementation specific. Eg pypy seems to produce an empty __dict__ after __new__ instead of None, and modifies test_module for this. So we might do well enough without the extra compatibility work, though i don't feel that strongly about it.

The test suite seemed to run without any new regressions when trying the patch on trunk. Absent further better informed feedback i'd favour committing to 2.3 at least.

- kz

Alright, if it's decided we don't want the null __dict__ behavior, I can create a new patch

Applied to trunk in r3157.  I removed the XXX commenting around the substitution of types.ModuleType for type(sys) since it seemed like a better solution, not a hack.

Added to 2.3 branch in r3161.