weak_dict.pyx is not python3 compatible

[aapitzsch]

The structure of PyDictObject used by weak_dict changed from Python 2 to Python 3, e.g PyDictEntry has been removed

REF:

• ​https://docs.python.org/2/c-api/dict.html
• ​https://docs.python.org/3/c-api/dict.html

[nbruin]

The change is significant. CPython3 still uses the same underlying logic, so that should still work, but it looks like it will be difficult to support both Py2 and Py3 with the same source. Do we have conditional compilation flags in cython for such cases? I imagine the transition is smoothest if we can ensure that we can support Py2 and Py3 from a single source, at least for a while.

If I recall correctly: PyDict_GetItemWithError is available in Py3.

For the rest, it should just be del_dictitem_by_exact_value that needs rewriting to account for the renaming of certain structures and, most importantly, dealing with split tables (which is new) as well as combined tables (which are basically the old dict lay-out).

It may be worth checking if the special trick used to participate in the Python Trashcan is still necessary with Py3.

Iteration protection has changed in Py3 as well a bit, so perhaps we don't quite need the protection we're enforcing in deletion anymore.

I think presently the "contains" test also reaches into the internal structure. However, "contains" doesn't really produce verifiable results on a WeakValueDict anyway, because the member can always disappear between the containment test and retrieval, so we could not override that routine (in the same way that "len" is not overridden).

For me the main issues that prevent me from fixing this issue are that I don't have access to a platform where I can compile cython against python 3 and that I don't know how to make cython source compile conditionally wrt. Py2 vs. Py3. It's probably easier if someone who does know how to deal with those things makes the necessary adjustments. It's not rocket science, but you do have to carefully read dictobject.c

If we could somehow link to ​delitem_common the deletion would almost be taken care of. I'm afraid that's not an exported symbol, though, so we basically have to replicate it.

Also, perhaps we should always force the WeakValueDict? to be "combined" (the old-style layout), because split tables don't allow deletion, and triggering reshaping of the dictionary in a weakref callback is probably not a good idea.

[chapoton]

build failure looks like that:

[sagelib-7.6.beta4] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c: In function '__pyx_f_4sage_4misc_9weak_dict_PyDict_GetItemWithError':
[sagelib-7.6.beta4] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1393:3: error: unknown type name 'PyDictEntry'
[sagelib-7.6.beta4]    PyDictEntry *__pyx_v_ep;
[sagelib-7.6.beta4]    ^
[sagelib-7.6.beta4] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1398:3: error: unknown type name 'PyDictEntry'
[sagelib-7.6.beta4]    PyDictEntry *__pyx_t_2;
[sagelib-7.6.beta4]    ^
[sagelib-7.6.beta4] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1419:25: error: 'PyDictObject {aka struct <anonymous>}' has no member named 'ma_lookup'

[chapoton]

This is one of our cython+python3 compilation problems.

[chapoton]

Could somebody do some action on the problem here, please ?

[chapoton]

This is probably the last cython file that does not compile with python3 !!

[nbruin]

Is there an easy way to get a "cython+python3" setup on which I can try weak_dict? I do recall how I made it work on py2, so I might be in a reasonable position to adapt it to py3.

Do we need a way to switch between the two? I don't think it will be so easy to make a version that works on both py2 and py3 (given that we reach into dicts beyond what's exposed by the API)

[nbruin]

OK, I've made some headway here. The changes are quite manageable. However, we do need the content and memory layout of the PyDictKeysObject. That's not part of the API and the layout of the PyDictKeysObject struct isn't even exported in Python.h (it's in dict-common.h, which is an internal, non-exported header file in CPython.).

As before, this means that this module might need rewriting if CPython decides to change the memory layout (and/or its hash probing).

The bad news is that they have done exactly that from 3.5 to 3.6 (they added order-preserving properties to dictionaries). So: do we have a particular target we're aiming for with sage? 3.5 vs. 3.6 makes a significant difference (larger than 2.7 vs. 3.5 actually)

Of course, reaching into CPython beyond the official CAPI is questionable. The alternative is to reimplement the entirety of dict in order to have WeakValueDictionary. Either that or try to get this thing accepted up-stream (after all, the WeakValueDictionary that ships with python (also the one with python 3) has demonstrable problems). Are we comfortable with the added maintenance burden of a module that depends on more than just the CAPI of CPython?

[nbruin]

OK, I've attached a version of weak_dict.pyx that compiles on python 3.6 and that seems to work too. Some more testing should be done and someone should look into how we can merge this into sage without breaking it on python 2.7

[nbruin]

Python3.6 compatible version of weak_dict.pyx

[nbruin]

Some updates. Main thing is that the default lookup function doesn't expect deleted entries, so we should trigger the dict to use the more general lookup.

There is a dictionary variant that has "split tables" (for keys and values, so that it can share the keys between different instances). These should only occur as instance dictionaries of objects so they shouldn't occur for WeakValueDictionaries. These don't support deletions either.

[nbruin]

Ping to chapoton. Did you need a fix urgently? The file attached here does solve the compilation issue on Python3.6.

[chapoton]

i have no available time this week

[chapoton]

I made a git branch with your latest patch.

---

New commits:

​02a2b13

py3.6 compatible weak dictionaries

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​787975e

trac 22305 adding the new weak_dict.pyx

[chapoton]

I made a git branch with your latest patch.

EDIT: it was not cythonizing correctly because of a typo. Now corrected in latest commit.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​8722eb7

trac 22305 typo

[chapoton]

next step here would be to have the old code for python2 and the new for python3. I have no idea how to do that.

[nbruin]

Replying to chapoton:

next step here would be to have the old code for python2 and the new for python3. I have no idea how to do that.

Yes, I looked into that and came up with nothing either. I got the impression that the setup.py file would be the place to set a flag depending on the target, and then use "IF" in cython to do a conditional compile. We could separate out the code for delete_by_exact_value into a separate file if that makes things a little cleaner. That's the only routine that depends on the implementation details of dict.

It looks like the Cython IF is a little more restricted that the C preprocessor one. I tried IF PY_HEX_VERSION > 0x03060000: but couldn't get it to work.

[chapoton]

@jdemeyer, do you know how to achieve properly this kind of py2/py3 switch in cython code ?

[jdemeyer]

I'll have a look.

[jdemeyer]

Besides: copying all that stuff from Python's sources is really bad style. I would much prefer to patch Python to make whatever you need public.

[jdemeyer]

I am not convinced that we need completely separate files for Python 2 and Python 3. There would be a lot of code duplication and that is bad style.

Some changes for Python 3 already make sense in Python 2 (just to give one example: replacing long by Py_hash_t).

[jdemeyer]

My proposal would be:

1. Make a separate ticket to deal with changes to weak_dict.pyx which are good to do anyway regardless of Python vesion.

2. Patch Python 3 to make public whatever is needed for the Python 3 version of weak_dict.pyx.

3. Finally, use conditional compilation to deal with Python 2/3 differences in weak_dict.pyx.

[nbruin]

Number 2 is a useful exercise in trying to see if we can get a patch submitted upstream, but realistically I don't think it's very realistic that it will be accepted, given that "deleting an item from a dict by hash and exact value" isn't something that fits in the dict data abstraction very well, and might be hard to realize on other implementations than CPython.

Requiring a patched python 3 for sage would really be a step backwards in terms of making sage available via other means than its own distribution packaging, so I don't think we should do that.

I agree we can split out differences quite a bit. Basically, it's just the implementation of delete_by_exact_value that will depend on the python version, so we could park that in a separate file at least. We're going to need to differentiate *some* cython compilation on python version, though, and currently I don't know how to achieve that. (that's point 3). Can we clear that up? It shouldn't be necessary to program this stuff directly in C, just to have access to PY_HEX_VERSION ...

[chapoton]

ping ? Jeroen, if you want to make a ticket for your point 1, please do so.

[nbruin]

The mechanism to pass compile-time variables to cython (for use in IF ...) seems to be something along the lines of:

from distutils.core import setup
from Cython.Build import cythonize
from sys import version_info

if version_info.major <= 2:
    env = {'PYTHON_VERSION': 2}
elif version_info.major == 3 and version_info.minor >= 6:
    env = {'PYTHON_VERSION': 3}
else:
    raise RuntimeError("Unsupported version")

setup(
    name = 'weak_dict',
    ext_modules = cythonize("weak_dict.pyx",compile_time_env=env),
)

I have no idea where put this in sage, though. I haven't been able to find the "setup" command that is relevant for "weak_dict".

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​7b4ec6c	Add sys.version_info as "PYTHON_VERSION" to cython compile-time environment
​4c73dbe	tract #22305 -- split off del_dictitem_by_exact_value and comile conditionally based on Py2/Py3

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​71b68d2

tract #22305 -- split off del_dictitem_by_exact_value and compile conditionally based on Py2/Py3

[nbruin]

It seems we can simply add sys.hexversion as PY_VERSION_HEX (which is the CAPI name of the variable) to the compile-time environment for all sage cythonization. That should be fairly low-cost.

I have split off the python-version-sensitive code into a separate file, which seems to work fine. Code replication is now a lot more limited. If someone sees an economic way of sharing the docstring, that would be great. On the other hand, these two implementations are so different that having different docs (and tests?) for them might make sense.

I have tested compiling it within sage on Py2.7 and directly on Python3.6 via the following setup.py:

from distutils.core import setup
from Cython.Build import cythonize
import sys

setup(
    name = 'weak_dict',
    ext_modules = cythonize(["weak_dict.pyx","dict_del_by_value.pyx"],compile_time_env={'PY_VERSION_HEX':sys.hexversion}),
)

Since this routine breaks the abstraction of "PyDict?" I am pretty sure the code would not get accepted as an API change. The only thing that I could see happening is that python might accept the routine as an internal one and put WeakValueDict in C to use it.

The only option I see to do this "cleanly" outside of Python is to implement our own WeakValueDict from scratch, i.e., don't base it on dict. I don't think that is less maintenance burden than tracking python's dict, and certainly a larger change than the one on this ticket at this point in sage.

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​54d8c2d

Correction: Py3 does provide PyDict_GetItemWithError but Cython doesn't provide it yet

[chapoton]

seems to be ok on python2 sage, see patchbot report

I will try with SAGE_PYTHON3=yes

[chapoton]

hum, it seems that latest update to numpy broke the SAGE_PYTHON3 build..

[jhpalmieri]

See #22582 for a numpy upgrade which should fix the python 3 build.

[chapoton]

a tentative python3-compilation answers the following:

cdef del_dictitem_by_exact_value(PyDictObject *mp, PyObject *value, Py_hash_t ha
sh)
                                ^
------------------------------------------------------------

sage/misc/dict_del_by_value.pxd:5:33: Non-extern C function 'del_dictitem_by_exa
ct_value' declared but not defined

[nbruin]

Was that on <=python3.5 ? Presently the implementation of dictionaries in CPython 3.0 -- 3.5 is not supported, and as a result dict_del_by_value.pyx dutifully refrains from defining a routine.

[chapoton]

Yes, this is on the current python3 in sage, which is 3.5.1

So this means that we have to upgrade python3 package, right ?

[nbruin]

Replying to chapoton:

Yes, this is on the current python3 in sage, which is 3.5.1

So this means that we have to upgrade python3 package, right ?

Yes. I am surprised your earlier experiments worked then. If the code that was here before even compiled against 3.5, it would certainly segfault.

​https://docs.python.org/3/whatsnew/3.6.html#whatsnew36-compactdict

[chapoton]

I have created #22942

[chapoton]

ok, green bot, I am setting to positive.

[chapoton]

hmm, does not seem to compile with SAGE_PYTHON3=yes and python3.6.1

[chapoton]

log:

[sagelib-8.0.beta7] [1/1] gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/home/chapoton/sage3/local/include -I/home/chapoton/sage3/local/include/python3.6m -I/home/chapoton/sage3/local/lib/python3.6/site-packages/numpy/core/include -I/home/chapoton/sage3/src -I/home/chapoton/sage3/src/sage/ext -I/home/chapoton/sage3/src/build/cythonized -I/home/chapoton/sage3/src/build/cythonized/sage/ext -I/home/chapoton/sage3/local/include/python3.6m -c /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c -o build/temp.linux-x86_64-3.6/home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.o -fno-strict-aliasing
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c: In function '__pyx_f_4sage_4misc_9weak_dict_PyDict_GetItemWithError':
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1398:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_v_ep;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1403:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_t_2;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1424:25: error: 'PyDictObject {aka struct <anonymous>}' has no member named 'ma_lookup'
[sagelib-8.0.beta7]    __pyx_t_2 = __pyx_v_mp->ma_lookup(__pyx_v_mp, ((PyObject *)((void *)__pyx_v_key)), __pyx_t_1); if (unlikely(__pyx_t_2 == NULL)) __PYX_ERR(0, 152, __pyx_L1_error)
[sagelib-8.0.beta7]                          ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1444:25: error: request for member 'me_value' in something not a structure or union
[sagelib-8.0.beta7]      __pyx_r = __pyx_v_ep->me_value;
[sagelib-8.0.beta7]                          ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c: In function '__pyx_f_4sage_4misc_9weak_dict_init_dummy':
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1497:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_v_ep0;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1498:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_v_ep;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1503:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_t_2;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1538:25: error: 'PyDictObject {aka struct <anonymous>}' has no member named 'ma_table'
[sagelib-8.0.beta7]    __pyx_t_2 = __pyx_v_mp->ma_table;
[sagelib-8.0.beta7]                          ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1558:25: error: 'PyDictObject {aka struct <anonymous>}' has no member named 'ma_mask'
[sagelib-8.0.beta7]    __pyx_t_3 = __pyx_v_mp->ma_mask;
[sagelib-8.0.beta7]                          ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1588:29: error: request for member 'me_key' in something not a structure or union
[sagelib-8.0.beta7]      __pyx_t_6 = ((__pyx_v_ep->me_key != NULL) != 0);
[sagelib-8.0.beta7]                              ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1598:27: error: request for member 'me_key' in something not a structure or union
[sagelib-8.0.beta7]        __pyx_r = __pyx_v_ep->me_key;
[sagelib-8.0.beta7]                            ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c: In function '__pyx_f_4sage_4misc_9weak_dict_del_dictitem_by_exact_value':
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1655:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_v_ep0;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1656:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_v_ep;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1660:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_t_1;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1674:37: error: 'PyDictObject {aka struct <anonymous>}' has no member named 'ma_mask'
[sagelib-8.0.beta7]    __pyx_v_mask = ((size_t)__pyx_v_mp->ma_mask);
[sagelib-8.0.beta7]                                      ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1683:25: error: 'PyDictObject {aka struct <anonymous>}' has no member named 'ma_table'
[sagelib-8.0.beta7]    __pyx_t_1 = __pyx_v_mp->ma_table;
[sagelib-8.0.beta7]                          ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1711:27: error: request for member 'me_key' in something not a structure or union
[sagelib-8.0.beta7]    __pyx_t_2 = ((__pyx_v_ep->me_key == NULL) != 0);
[sagelib-8.0.beta7]                            ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1751:42: error: request for member 'me_value' in something not a structure or union
[sagelib-8.0.beta7]      __pyx_t_3 = ((((PyObject *)__pyx_v_ep->me_value) != __pyx_v_value) != 0);
[sagelib-8.0.beta7]                                           ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1757:29: error: request for member 'me_hash' in something not a structure or union
[sagelib-8.0.beta7]      __pyx_t_3 = ((__pyx_v_ep->me_hash != __pyx_v_hash) != 0);
[sagelib-8.0.beta7]                              ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1787:29: error: request for member 'me_key' in something not a structure or union
[sagelib-8.0.beta7]      __pyx_t_2 = ((__pyx_v_ep->me_key == NULL) != 0);
[sagelib-8.0.beta7]                              ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1839:54: error: request for member 'me_key' in something not a structure or union
[sagelib-8.0.beta7]    __pyx_t_5 = PyList_SetItem(__pyx_v_T, 0, __pyx_v_ep->me_key); if (unlikely(__pyx_t_5 == -1)) __PYX_ERR(0, 258, __pyx_L1_error)
[sagelib-8.0.beta7]                                                       ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1889:13: error: request for member 'me_key' in something not a structure or union
[sagelib-8.0.beta7]    __pyx_v_ep->me_key = __pyx_v_4sage_4misc_9weak_dict_dummy;
[sagelib-8.0.beta7]              ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1898:54: error: request for member 'me_value' in something not a structure or union
[sagelib-8.0.beta7]    __pyx_t_5 = PyList_SetItem(__pyx_v_T, 1, __pyx_v_ep->me_value); if (unlikely(__pyx_t_5 == -1)) __PYX_ERR(0, 263, __pyx_L1_error)
[sagelib-8.0.beta7]                                                       ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:1907:13: error: request for member 'me_value' in something not a structure or union
[sagelib-8.0.beta7]    __pyx_v_ep->me_value = NULL;
[sagelib-8.0.beta7]              ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c: In function '__pyx_pf_4sage_4misc_9weak_dict_19WeakValueDictionary_22__contains__':
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:4290:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_v_ep;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:4294:3: error: unknown type name 'PyDictEntry'
[sagelib-8.0.beta7]    PyDictEntry *__pyx_t_2;
[sagelib-8.0.beta7]    ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:4316:25: error: 'PyDictObject {aka struct <anonymous>}' has no member named 'ma_lookup'
[sagelib-8.0.beta7]    __pyx_t_2 = __pyx_v_mp->ma_lookup(__pyx_v_mp, ((PyObject *)((void *)__pyx_v_k)), __pyx_t_1); if (unlikely(__pyx_t_2 == NULL)) __PYX_ERR(0, 928, __pyx_L1_error)
[sagelib-8.0.beta7]                          ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:4326:26: error: request for member 'me_value' in something not a structure or union
[sagelib-8.0.beta7]    __pyx_t_4 = (__pyx_v_ep->me_value != NULL);
[sagelib-8.0.beta7]                           ^
[sagelib-8.0.beta7] /home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:4332:46: error: request for member 'me_value' in something not a structure or union
[sagelib-8.0.beta7]    __pyx_t_4 = (PyWeakref_GetObject(__pyx_v_ep->me_value) != Py_None);
[sagelib-8.0.beta7]                                               ^
[sagelib-8.0.beta7] error: command 'gcc' failed with exit status 1

[nbruin]

Are you testing the right branch? PyDictEntry doesn't occur in weak_dict.pyx. The only references to it are in dict_del_by_value.pyx and they don't get exported.

[chapoton]

hmm, indeed. I was trying the vanilla develop branch... sorry for the noise..

[chapoton]

Indeed, it works with SAGE_PYTHON3=yes, so I agree with the positive review.

[tscrim]

Ah, sorry, I didn't realize you were (re?)checking it. I saw comment:46 and thought you just forgot to flip it back to positive review.

[chapoton]

could the changes made here possibly explain the issue arising there:

Traceback (most recent call last):
  File "/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/rings/polynomial/polynomial_ring_constructor.py", line 604, in _get_from_cache
    return _cache[key]
  File "sage/misc/weak_dict.pyx", line 699, in sage.misc.weak_dict.WeakValueDictionary.__getitem__ (/home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:3507)
TypeError: unhashable type: 'ComplexIntervalField_class_with_category'

during an experimental python3-build of sage ?

[nbruin]

Replying to chapoton:

could the changes made here possibly explain the issue arising there:

Traceback (most recent call last):
  File "/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/rings/polynomial/polynomial_ring_constructor.py", line 604, in _get_from_cache
    return _cache[key]
  File "sage/misc/weak_dict.pyx", line 699, in sage.misc.weak_dict.WeakValueDictionary.__getitem__ (/home/chapoton/sage3/src/build/cythonized/sage/misc/weak_dict.c:3507)
TypeError: unhashable type: 'ComplexIntervalField_class_with_category'

during an experimental python3-build of sage ?

No.

Also, it's not such a good idea to ask questions on closed tickets. Probably no-one will read them, unless they happen to be looking at the ticket for reference for something else.