Issue2746

classification
Title: Divergence from Python in Java mapping types
Type: behaviour Severity: normal
Components: Core Versions:
Milestone:
process
Status: open Resolution: accepted
Dependencies: Superseder:
Assigned To: jeff.allen Nosy List: jeff.allen
Priority: normal Keywords: test failure causes

Created on 2019-03-24.14:32:32 by jeff.allen, last changed 2019-04-04.08:18:03 by jeff.allen.

Messages
msg12394 (view) Author: Jeff Allen (jeff.allen) Date: 2019-03-24.14:32:32
I beefed up test_dict_jy to test more thoroughly the behaviour of Java containers exposed as Python mapping types equivalent to dict(). This was initially just in aid of #2649, but revealed a few other warts.

At the time of writing, the beefed-up test is not in the repository, but I'll do that in the next few days, with skips in place citing this issue. I haven't analysed the causes yet, but the added tests produce:


======================================================================
ERROR: test_has_key (__main__.JavaHashtableDictTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Jeff\Documents\Eclipse-O\jython-jvm9\dist\Lib\test\test_dict.py", line 73, in test_has_key
    k.sort()
AttributeError: 'java.util.Hashtable$Enumerator' object has no attribute 'sort'

======================================================================
ERROR: test_repr_value_None (__main__.JavaHashtableDictTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Jeff\Documents\Eclipse-O\jython-jvm9\dist\Lib\test\test_dict_jy.py", line 265, in test_repr_value_None
    x = self._class({1:None})
NullPointerException: java.lang.NullPointerException

======================================================================
ERROR: test_has_key (__main__.JavaConcurrentHashMapDictTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Jeff\Documents\Eclipse-O\jython-jvm9\dist\Lib\test\test_dict.py", line 73, in test_has_key
    k.sort()
AttributeError: 'java.util.concurrent.ConcurrentHashMap$KeyIterator' object has no attribute 'sort'

======================================================================
ERROR: test_repr_value_None (__main__.JavaConcurrentHashMapDictTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Jeff\Documents\Eclipse-O\jython-jvm9\dist\Lib\test\test_dict_jy.py", line 265, in test_repr_value_None
    x = self._class({1:None})
NullPointerException: java.lang.NullPointerException

======================================================================
FAIL: test_keys (__main__.JavaHashtableDictTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Jeff\Documents\Eclipse-O\jython-jvm9\dist\Lib\test\test_dict.py", line 41, in test_keys
    self.assertEqual(d.keys(), [])
AssertionError: java.util.Collections$EmptyEnumeration@1e495414 != []

======================================================================
FAIL: test_keys (__main__.JavaConcurrentHashMapDictTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Users\Jeff\Documents\Eclipse-O\jython-jvm9\dist\Lib\test\test_dict.py", line 41, in test_keys
    self.assertEqual(d.keys(), [])
AssertionError: java.util.concurrent.ConcurrentHashMap$KeyIterator@62ea8931 != []
msg12412 (view) Author: Jeff Allen (jeff.allen) Date: 2019-03-30.21:43:18
I've mostly fixed these, except for a few I think may be unavoidable, and skipped with a clear conscience.

A lot of it was to do with the treatment of null: many places we have already adapted our coercion of Py.None so we do not ask maps to hold a Java null, but not everywhere. We missed some cases and the improved coverage has found (some of) them for us.

The remaining problem is with the keys() method in mapping types we allow to be treated as equivalent to dict (PyDictionary). The problem is that both ConcurrentHashMap and Hashtable have a keys() method. It returns an Enumerator.

We try to dress up mapping types with the Python mapping API, which includes a keys() method that returns a PyList. Hoever, where there is a clash with a pre-existing method, the Java meaning wins. We have noticed this problem already with LinkedList.pop() in #2645, raised by James. We decided there that letting Java win was the right answer: if an object in Jython claims to be a java.util.Hashtable, its keys() method should return what the Javadoc says.

However, I think these clashes call into question the proxy idea as presently offered. I think there is a case for saying that a java.lang.Hashtable should do exactly and only what it says on the tin, and if you want it to behave like a dict, then there's an explicit wrapper that presents it as exactly a dict, so code that tests for that will find it behaves correctly.
msg12414 (view) Author: Jeff Allen (jeff.allen) Date: 2019-03-31.07:40:18
Second thoughts on this, since I find test_dict2java now fails. I think I have learned:

1. PyDictionary uses a Map internally that stores PyObjects, but offers a Java Map interface (to Java clients of the PyDictionary) through which these internal PyObjects appear as Java equivalents in put and get operations. (Clever.) In particular, a Py.None internally maps to/from a null externally.

2. In a proxy for a Java Map, the backing Map stores Java objects, and Java put and get operations should operate on the backing Map directly, while Python __setitem__ and __getitem__ involve translation from Python to Java, and Java to Python respectively. In particular, a Py.None on the Python side becomes null to the Java Map, and it's just too bad if the Map does not accept nulls.

Others before me have tried to work around this rejection of nulls by excepting Py.None from conversion on the way in. My first solution was to do that more consistently, but the result is that PyDictionary has to do it too in its Java interface, so that putAll does not receive a null in the constructor of the proxied Map type. But then an otherwise pure Java data structure ends up containing this one kind of PyObject.

I think the consistency should actually be the other way around: operations that try to store None in a proxied data structure where null is not accepted should consistently fail. I see this as another expression of the choice to let types that claim a "Java name" behave as themselves, not as the Python types they resemble.
msg12418 (view) Author: Jeff Allen (jeff.allen) Date: 2019-04-01.21:51:46
The more I try to increase test coverage, the more dubious I get about the way we have added the Python mapping API to Java Map objects.

I've noticed that our modified version of test_dict, while it uses an instance of the type under test some of the time, obtained from self._make_dict(), where it refers to the type itself, still refers to dict. This happens particularly where it needs to define a sub-class of the type under test. Or rather used to. But correcting this leads to many more test failures.

Sub-classing works really badly: test_fromkeys uses subclassing extensively, but the overrides of __new__ and __setitem__ do not get called. This should probably not be a surprise, as there is no *Derived class to hook the special methods. At least, I don't think there is.
msg12422 (view) Author: Jeff Allen (jeff.allen) Date: 2019-04-04.08:18:03
This improves things a lot, but I'm not sure I'd go as far as "fixed":
https://hg.python.org/jython/rev/65c4b774a715

Surely proxied set and list types need similar treatment to map.
History
Date User Action Args
2019-04-04 08:18:03jeff.allensetmessages: + msg12422
2019-04-01 21:51:47jeff.allensetmessages: + msg12418
2019-03-31 07:40:18jeff.allensetmessages: + msg12414
2019-03-30 21:43:18jeff.allensetassignee: jeff.allen
resolution: accepted
messages: + msg12412
2019-03-24 14:32:32jeff.allencreate