fix symbolic/pynac.pyx doctests

[rws]

Several doctests in symbolic/pynac.pyx iterate over range(get_ginac_serial(), get_ginac_serial()+100). You guess it (sigh), 100 is arbitrary and, right now with some new tickets that introduce new functions, it has become too small, leading to unrelated doctest fails in symbolic/pynac.pyx. This ticket should make sure that this does not happen again.

[kcrisman]

I agree that this is worth fixing permanently, but unless there is an obvious fix we shouldn't let this hold up adding more symbolic functions.

[rws]

Replying to kcrisman:

I agree that this is worth fixing permanently, but unless there is an obvious fix we shouldn't let this hold up adding more symbolic functions.

This is not really difficult.

The central database is a dictionary globally defined in symbolic/function.pyx named sfunction_serial_dict. It has pairs with key of type unsigned and values of Function objects (which can be GinacFunctions or BuiltinFunctions). Accordingly this database is filled with all known Pynac functions (which have the GinacFunction façade) first and then those defined via BuiltinFunction. The unsigned key is the serial number which, on each new function registration, is incremented by one, namely in GiNaC::function::register_new()(​https://github.com/pynac/pynac/blob/master/ginac/function.cpp#L1445) by just adding to the Pynac registry and reporting its size minus one. After the GiNaC functions have been added, the global GINAC_FN_SERIAL is set which is then accessed via get_ginac_serial() to get the start key of the Sage defined functions.

There is a problem if ever a function is deregistered. Can this happen? No mechanism in Pynac exists for this at the moment, so it's not a practical consideration.

So the solution is easy: instead of simply adding 100 to the start point, we try all keys from the start key to the latest key used---this need not be a static variable set on registration. Since functions are never deregistered the latest is, again, just the size of the GiNaC registry minus one. This can be had from a call to g_registered_functions().size() in symbolic/pynac.pyx.

cdef get_fn_serial_c():
    return g_registered_functions().size()

def get_fn_serial():
    return get_fn_serial_c()

sage: from sage.symbolic.pynac import get_fn_serial
sage: get_fn_serial()
127
sage: from sage.symbolic.pynac import get_ginac_serial
sage: get_ginac_serial()
40
sage: from sage.symbolic.function import get_sfunction_from_serial
sage: for i in range(get_ginac_serial(), get_fn_serial()):
    print get_sfunction_from_serial(i)
... // prints all known functions
sage: print get_sfunction_from_serial(get_fn_serial()+1)
None

[rws]

As they say the first thing that goes down the drain is the planning. The above solution does not account for user defined Python functions that do not inherit from BuiltinFunction or Function.

[rws]

No, that was an off-by-one error. Please review.

---

New commits:

​13ba99d

18257: fix symbolic/pynac.pyx doctests

[kcrisman]

Trivial - duplicate lines in test

+        sage: from sage.symbolic.pynac import get_fn_serial
+        sage: from sage.symbolic.function import get_sfunction_from_serial
+        sage: from sage.symbolic.pynac import get_fn_serial

This patch seems reasonable. What was comment:3 about? I sense that it was a red herring, but if it could be useful for test-driving this I would love to try it. I assume that since this isn't actually called in Sage but is really just for doctesting, it shouldn't have any speed implications.

[git]

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

​52ccd47

18257: fix typo

[rws]

Replying to kcrisman:

... What was comment:3 about? I sense that it was a red herring, but if it could be useful for test-driving this I would love to try it. I assume that since this isn't actually called in Sage but is really just for doctesting, it shouldn't have any speed implications.

It's taken care of in the doctests.

[kcrisman]

I'm not sure why but I still get

sage -t src/sage/symbolic/pynac.pyx
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 512, in sage.symbolic.pynac.py_latex_function_pystring
Failed example:
    get_sfunction_from_serial(i) == foo
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 514, in sage.symbolic.pynac.py_latex_function_pystring
Failed example:
    py_latex_function_pystring(i, (x,y^z))
Expected:
    'my args are: x, y^z'
Got:
    '\\mathrm{bar}\\left(x, y^{z}\\right)'
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 603, in sage.symbolic.pynac.py_print_fderivative_for_doctests
Failed example:
    get_sfunction_from_serial(i) == foo
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 605, in sage.symbolic.pynac.py_print_fderivative_for_doctests
Failed example:
    py_print_fderivative(i, (0, 1, 0, 1), (x, y^z))
Expected:
    D[0, 1, 0, 1]func_with_args(x, y^z)
Got:
    D[0, 1, 0, 1](foo)(x, y^z)
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 665, in sage.symbolic.pynac.py_latex_fderivative_for_doctests
Failed example:
    get_sfunction_from_serial(i) == foo
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 667, in sage.symbolic.pynac.py_latex_fderivative_for_doctests
Failed example:
    py_latex_fderivative(i, (0, 1, 0, 1), (x, y^z))
Expected:
    D[0, 1, 0, 1]func_with_args(x, y^z)
Got:
    D[0, 1, 0, 1]\left(\mathrm{bar}\right)\left(x, y^{z}\right)
**********************************************************************

when I have both this ticket and #15024 together. Did I do something wrong? (Note: this is based off of 6.8.beta1, if it matters.)

[rws]

Cannot confirm in a first attempt (I branched from #18257, merged #15024 with git trac pull, then merged develop).

[rws]

Same branch does work with pynac-0.3.9 (in case you left that installed and that was the difference between us).

[rws]

Your line numbers also don't match #18257.

[kcrisman]

Okay, I'll trash the branches I made locally and try again - may not be immediately, I'm at a conference.

[kcrisman]

I must have either not merged this one or perhaps in the wrong order or something... Anyway, all systems are go, and as expected #15024 fails without this and works with it. Great!

[rws]

Hold on. I have a necessary addition because of a commit in Pynac where I removed the registry entries for some defunct functions which makes two of the doctest here fail. This would fit in nicely here.

[git]

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

​557badf

adjust doctest because of upstream changes

[rws]

Could you please have a last quick look?

[kcrisman]

I don't understand why this fails, though? (I mean it shouldn't currently, nothing failed for me.) Surely it would be better to put that commit with the next Pynac upgrade ticket? Easier for the reviewer to test, then (i.e. first upgrade Pynac, run tests (fail), then add branch, watch tests pass).