Map._extra_slots(): do not pass dict

[jdemeyer]

In the Map method _extra_slots, a dict is passed as input. That dict is then modified in-place and also returned as output.

This awkward logic is not needed for anything. We simplify this by not passing a dict as input and creating a new dict from scratch if needed.

Component: coercion

Author: Jeroen Demeyer

Branch/Commit: 375e40f

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/24372

[jdemeyer]

Branch: u/jdemeyer/ticket/24372

[jdemeyer]

New commits:

375e40f

Map._extra_slots(): do not pass dict

[jdemeyer]

Commit: 375e40f

[tscrim]

comment:3

LGTM overall, but I do not feel 100% comfortable relying on an uninitialized zero_element being None by default:

        if self.zero_element is None:
            self.zero_element = self._codomain.zero()

So I would add self.zero_element = None in the __init__.

Probably we should also add the other cdef classes (such as Z_to_quadratic_field_element) to the .pxd file, but that can be done on a followup.

[jdemeyer]

comment:4

Interestingly, http://cython.readthedocs.io/en/latest/src/userguide/special_methods.html#initialisation-methods-cinit-and-init says

"Any Python attributes have also been initialised to None, but you probably shouldn’t rely on that."

[tscrim]

comment:5

So are you saying we should just leave it be?

[jdemeyer]

comment:6

Replying to @tscrim:

So are you saying we should just leave it be?

I just sent an email to the Cython mailing list to ask for clarification.

[jdemeyer]

comment:7

Reply from a Cython developer:

As long as Cython controls the type hierarchy, you can rely on it. Exposing
NULL values for Python attributes to !Python/Cython code would be unhelpful.

There is one exception: when __cinit__() is called, attributes that are
defined by subtypes have not been initialised yet and thus are still NULL
instead of None.

In short: what I did is correct.

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:8

Alright, then positive review.

[vbraun]

Changed branch from u/jdemeyer/ticket/24372 to 375e40f