Convert python interface to jinja2

[dschwoerer]

Should make it easier to work with the code.

[ZedThree]

This is nice, thanks! I wonder if we can go even further though and try to move as much code out of the generation loops as possible?

For example, the different extension classes for the Fields could inherit from a base class that handles most/all of the common functionality, even if it's just the non-generated parts.

There's also places with loops that could be done in Python, e.g. :

    def _setmembers(self):
{% for f in "dx", "dy", "J", "Bxy", "g11", "g22", "g33", "g12", "g13", "g23", "g_11", "g_22", "g_33", "g_12", "g_13", "g_23", "G1_11", "G1_22", "G1_33", "G1_12", "G1_13", "G1_23", "G2_11", "G2_22", "G2_33", "G2_12", "G2_13", "G2_23", "G3_11", "G3_22", "G3_33", "G3_12", "G3_13", "G3_23", "G1", "G2", "G3", "ShiftTorsion", "IntShiftTorsion" %}
        self.{{f}} = f2dFromPtr(&self.cobj.{{f}})
{% endfor %}

could be something like

    def _setmembers(self):
        for f in ["dx", "dy", "J", "Bxy", "g11", "g22", "g33", "g12", "g13", "g23", "g_11", "g_22", "g_33", "g_12", "g_13", "g_23", "G1_11", "G1_22", "G1_33", "G1_12", "G1_13", "G1_23", "G2_11", "G2_22", "G2_33", "G2_12", "G2_13", "G2_23", "G3_11", "G3_22", "G3_33", "G3_12", "G3_13", "G3_23", "G1", "G2", "G3", "ShiftTorsion", "IntShiftTorsion"]:
        setattr(self, f) = f2dFromPtr(&getattr(self.cobj, f))

That's maybe not such a great example -- I'm not sure how the Cython syntax interacts with the Python there.

The next step would be to pull out all the non-jinja parts into separate modules, so that they're even easier to work with.

[dschwoerer]

The Fields are cdef'ed - thus I think it will not work.

But yes, I think splitting up the files, would already be nice, and then some parts wouldn't have to be jinja anyway.

[dschwoerer]

For example, the different extension classes for the Fields could inherit from a base class that handles most/all of the common functionality, even if it's just the non-generated parts.

I don't see the advantage. I would rather have everything defined in one place. If black could handle plain .pyx - that would be a different thing.

[ZedThree]

Awesome, LGTM! The build is also now much less noisy, though I still see a couple of warnings:

[142/147] Cythonizing python interface
warning: resolve_enum.pxd:49:21: 'deflt' redeclared 
warning: resolve_enum.pxd:189:24: 'Standard' redeclared 
warning: resolve_enum.pxd:223:15: 'Standard' redeclared

[dschwoerer]

It seems the namespace argument is not working as I hoped:

cdef extern from "bout_types.hxx" namespace "CELL_LOC":
    cdef CELL_LOC deflt
--
cdef extern from "bout_types.hxx" namespace "DIFF_METHOD":
    cdef DIFF_METHOD deflt
--
...
--
cdef extern from "bout_types.hxx" namespace "YDirectionType":
    cdef YDirectionType Standard
--
cdef extern from "bout_types.hxx" namespace "ZDirectionType":
    cdef ZDirectionType Standard
--
cdef extern from "bout_types.hxx" namespace "STAGGER":
    cdef STAGGER C2L
--
cdef extern from "bout_types.hxx" namespace "DERIV":
    cdef DERIV Standard
--
cdef extern from "other_enums.hxx" namespace "BRACKET_METHOD":
    cdef BRACKET_METHOD standard

Looking at the generated C++ code, it seems all good, and the correct type is used:

  /* "resolve_enum.pxd":15                                                                                                                                     
 * cdef inline CELL_LOC resolve_cell_loc(str):                                                                                                                 
 *     opts={                                                                                                                                                  
 *         "deflt":<int>deflt,             # <<<<<<<<<<<<<<                                                                                                    
 *         "CELL_LOC_deflt":<int>deflt,                                                                                                                        
 *         "centre":<int>centre,                                                                                                                               
 */
  __pyx_t_1 = __Pyx_PyDict_NewPresized(26); if (unlikely(!__pyx_t_1)) __PYX_ERR(2, 15, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_1);
  __pyx_t_2 = __Pyx_PyInt_From_int(((int)DIFF_METHOD::deflt)); if (unlikely(!__pyx_t_2)) __PYX_ERR(2, 15, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_2);
  if (PyDict_SetItem(__pyx_t_1, __pyx_n_u_deflt, __pyx_t_2) < 0) __PYX_ERR(2, 15, __pyx_L1_error)
  __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0;

I am thus inclined to just ignore it. Especially as the Standard / deflt - even if Cython would not be getting it right - would still have the same integer value.

It might make sense to change deflt (and standard to Standard - but that would be API breaking -.-) (but that would mean more errors)

If you think this is an issue I can open an issue with Cython, but I am inclined to ignore the warning.

[ZedThree]

It would definitely be nice to not have the warnings, but we still have a bunch of other warnings to fix too.