Opened 5 years ago

Last modified 4 years ago

#17872 needs_work defect

SingularKernelFunction documentation doesn't work

Reported by: Snark Owned by:
Priority: minor Milestone: sage-6.6
Component: documentation Keywords:
Cc: Merged in:
Authors: Julien Puydt Reviewers:
Report Upstream: N/A Work issues:
Branch: u/malb/t17872_singular_function_doc (Commits) Commit: 7fac2db140fd73567785d08af5af7cd18af82977
Dependencies: Stopgaps:

Description

I found the following during my debian experiments (sage's develop branch), but confirmed it using a vanilla sage 6.5 :

sage: from sage.algebras.letterplace.free_algebra_element_letterplace import poly_reduce
sage: help(poly_reduce)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-3-6f8ca6115b81> in <module>()
----> 1 help(poly_reduce)

/home/jpuydt/sage-6.5/local/lib/python2.7/site-packages/sage/misc/sagedoc.pyc in help(module)
   1455             from sage.misc.sageinspect import sage_getdef, _sage_getdoc_unformatted
   1456             docstr = 'Help on ' + str(module) + '\n'
-> 1457             docstr += 'Definition: ' + module.__name__ + sage_getdef(module) + '\n'
   1458             pydoc.pager(docstr + _sage_getdoc_unformatted(module))
   1459         else:

AttributeError: 'sage.libs.singular.function.SingularKernelFunction' object has no attribute '__name__'

Change History (12)

comment:1 Changed 5 years ago by vdelecroix

Note that

sage: from sage.algebras.letterplace.free_algebra_element_letterplace import poly_reduce
sage: poly_reduce??
Type:            SingularKernelFunction
String form:     NF (singular function)
File:            /opt/sage_flatsurf/local/lib/python2.7/site-packages/sage/libs/singular/function.so
Definition:      poly_reduce(self, ring=None, interruptible=True, attributes=None, *args)
Source:
cdef class SingularKernelFunction(SingularFunction):
    """
    EXAMPLES::
...

does work

comment:2 Changed 5 years ago by Snark

@vdelecroix: It works because it doesn't try to use the name, does it?

What I find strange is that a name is set in function.pyx... (line 1156).

comment:3 Changed 5 years ago by Snark

Ok, I tried to study how things work : in src/sage/misc/sagedoc.py line 1477, the code just tries to read poly_reduce._ _name_ _, which doesn't exist. What bothers me is that in src/sage/libs/singular/function.pyx, if I add "self._ _name_ _ = name" here and there, I actually get *more* errors about _ _name_ _ not making sense!

I must admit that python classes aren't my cup of tea...

comment:4 Changed 5 years ago by Snark

  • Branch set to u/Snark/singularkernelfunction_documentation_doesn_t_work

comment:5 Changed 5 years ago by Snark

  • Authors set to Julien Puydt
  • Commit set to 4c41c1fa8c35c41d94ceaca86d17f49b630cda45
  • Status changed from new to needs_review

Ok, I pushed a trivial patch which does the minimal amount of change... I'm pretty sure the right way would be to actually add the name somewhere, but since I don't know how to do that...


New commits:

4c41c1fSimple fix -- too simple, perhaps?

comment:6 Changed 5 years ago by jhpalmieri

Is this approach any better? {{{diff --git a/src/sage/algebras/letterplace/free_algebra_element_letterplace.pyx b/src/sage/algebras index d275963..a3cc1e2 100644 --- a/src/sage/algebras/letterplace/free_algebra_element_letterplace.pyx +++ b/src/sage/algebras/letterplace/free_algebra_element_letterplace.pyx @@ -22,8 +22,8 @@ from sage.rings.polynomial.multi_polynomial_ideal import MPolynomialIdeal

# Define some singular functions lib("freegb.lib")

-poly_reduce = singular_function("NF") -singular_system=singular_function("system") +poly_reduce = singular_function("NF", custom_name='poly_reduce') +singular_system=singular_function("system", custom_name='singular_system')

##################### # Free algebra elements

diff --git a/src/sage/libs/singular/function.pxd b/src/sage/libs/singular/function.pxd index 02937eb..ec8c752 100644 --- a/src/sage/libs/singular/function.pxd +++ b/src/sage/libs/singular/function.pxd @@ -68,6 +68,7 @@ cdef class KernelCallHandler?(BaseCallHandler?):

cdef class SingularFunction?(SageObject?):

cdef object _name

+ cdef public custom_name

cdef MPolynomialRing_libsingular _ring cdef BaseCallHandler? call_handler

diff --git a/src/sage/libs/singular/function.pyx b/src/sage/libs/singular/function.pyx index 4a885df..f2a9ceb 100644 --- a/src/sage/libs/singular/function.pyx +++ b/src/sage/libs/singular/function.pyx @@ -1568,7 +1568,7 @@ cdef class SingularKernelFunction?(SingularFunction?):

return cmd_n != -1

-def singular_function(name): +def singular_function(name, custom_name=None):

""" Construct a new libSingular function object for the given name.

@@ -1754,9 +1754,14 @@ def singular_function(name):

cdef SingularFunction? fnc try:

+ f = SingularKernelFunction?(name)

except NameError?:

+ f = SingularLibraryFunction?(name) + if custom_name is not None: + f.custom_name = custom_name + else: + f.custom_name = name + return f

def lib(name):

"""

diff --git a/src/sage/misc/sagedoc.py b/src/sage/misc/sagedoc.py index d024938..bae7f42 100644 --- a/src/sage/misc/sagedoc.py +++ b/src/sage/misc/sagedoc.py @@ -1474,7 +1474,10 @@ def help(module=None):

if hasattr(module, '_sage_doc_'):

from sage.misc.sageinspect import sage_getdef, _sage_getdoc_unformatted docstr = 'Help on ' + str(module) + '\n'

  • docstr += 'Definition: ' + module.name + sage_getdef(module) + '\n'

+ if hasattr(module, 'custom_name'): + docstr += 'Definition: ' + module.name + sage_getdef(module) + '\n' + elif hasattr(module, 'name'): + docstr += 'Definition: ' + module.custom_name + sage_getdef(module) + '\n'

pydoc.pager(docstr + _sage_getdoc_unformatted(module))

else:

python_help(module)

}}} It's untested and may very well not be the right thing to do...

Version 0, edited 5 years ago by jhpalmieri (next)

comment:7 Changed 5 years ago by Snark

Hmmm... that adds a new custom name to the objects, and adds code to use that in the documentation, so modifies things in two places. Isn't it possible to just add the name the documentation code already expects?

comment:8 follow-up: Changed 5 years ago by jhpalmieri

With code like

poly_reduce = singular_function("NF")

I'm not sure how to recover the string "poly_reduce".

comment:9 in reply to: ↑ 8 Changed 5 years ago by vdelecroix

Replying to jhpalmieri:

With code like

poly_reduce = singular_function("NF")

I'm not sure how to recover the string "poly_reduce".

I am not sure what you want to achieve... but

sage: from sage.algebras.letterplace.free_algebra_element_letterplace import poly_reduce
sage: from sage.misc.dev_tools import find_object_modules
sage: find_object_modules(poly_reduce)
{'sage.algebras.letterplace.free_algebra_element_letterplace': ['poly_reduce']}

On the other hand, adding a custom attribute to fix a special case is bad in my opinion. I find the approach used by Julien better.

Vincent

comment:10 Changed 4 years ago by malb

  • Branch changed from u/Snark/singularkernelfunction_documentation_doesn_t_work to u/malb/t17872_singular_function_doc
  • Commit changed from 4c41c1fa8c35c41d94ceaca86d17f49b630cda45 to 5dea008c123fde440eac6e1c5e849348619a059e

Isn't the best solution to just add __name__ to SingularFunction objects? I've created a branch with that fix applied and it seems to be working fine.


New commits:

5dea008SingularFunction._name → SingularFunction.__name__

comment:11 Changed 4 years ago by git

  • Commit changed from 5dea008c123fde440eac6e1c5e849348619a059e to 7fac2db140fd73567785d08af5af7cd18af82977

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

7fac2dbDoctest for SingularFunction.__name__ fix

comment:12 Changed 4 years ago by chapoton

  • Status changed from needs_review to needs_work

does not apply

Note: See TracTickets for help on using tickets.