src/sage/interfaces/singular.py: use GNU Info to read Singular's info #38899

orlitzky · 2024-10-31T23:01:48Z

Our Singular interface currently contains a hand-written parser for Singular's "info" file. This commit eliminates the custom parser in favor of launching GNU Info. GNU Info (or its superset, Texinfo) are widespread, portable, and easy to install on all of the systems we support, so in most cases this should be a "free" improvement.

The hand-written parser has several drawbacks:

The extra code is a maintenance burden. We should not be wasting our time reimplementing standard tools.
The custom parser is buggy. For example, it is supposed to raise a KeyError when documentation for a non-existent function is requested. However, the parser does not keep track of what section it's in, so, for example, get_docstring("Preface") returns the contents of the preface even though "Preface" is not a Singular function.
The first time documentation is requested, the entire info file is loaded into a dictionary. This wastes a few megabytes of memory for the duration of the Sage session.
The custom parser does not handle compression (GNU Info does transparently), and the end user or people packaging Singular may not be aware of that. If the system installation of Singular has a compressed info file, Sage won't be able to read it.

For contrast, the one downside to using GNU Info is that it adds a new runtime dependency to sagelib. To mitigate that, we do not technically require it, and instead raise a warning if the user (a) tries to read the Singular documentation and (b) has managed to find a system without GNU Info. Our singular_console() itself tries to launch GNU Info to display its interactive help, so the additional optional dependency is not so additional except in corner cases, such as a pypi installation of a subset of Sage linked against libsingular but without a full Singular install.

This was originally #32242

Since then,

GNU Info has become a standard package in the sage distribution
I'm now using sage.features.info to detect the "info" program, so nothing bad happens if it's not installed

Fixes #32242

github-actions · 2024-10-31T23:50:29Z

Documentation preview for this PR (built with commit 0275387; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

src/sage/interfaces/singular.py

kwankyu · 2024-11-07T11:44:33Z

Is this the expected result for unknown function?

kwankyu · 2024-11-07T11:46:37Z

>>`>>

seems broken parts... No?

kwankyu · 2024-11-07T11:53:01Z

On your branch

but previously it was

kwankyu · 2024-11-07T12:00:34Z

Perhaps I am not testing your branch on a proper platform... How can I invoke the info interface like

within sage?

orlitzky · 2024-11-07T16:32:10Z

You should just be able to run info if it's already installed on your system. To browse the singular help, you should be able to run info singular.info.

Can you please run info --version for me? I think your output may be different due to an older version. Sage comes with a new version but it will also accept an old version from the system, and there is an important bugfix that I am relying on to keep the parsing simple.

orlitzky · 2024-11-07T16:35:31Z

Is this the expected result for unknown function?

No, but I think it may be the case with info 6.x. If so, I'll need to handle that.

orlitzky · 2024-11-07T19:15:20Z

I updated both the feature and the spkg-configure.m4 to check for v7.0 or greater. I think that should fix all of the issues you're seeing. With v6.x, info would return "success" even if the section you want to read does not exist.

kwankyu · 2024-11-07T22:04:08Z

You should just be able to run info if it's already installed on your system. To browse the singular help, you should be able to run info singular.info.

Can you please run info --version for me? I think your output may be different due to an older version. Sage comes with a new version but it will also accept an old version from the system, and there is an important bugfix that I am relying on to keep the parsing simple.

$ sage -sh

Starting subshell with Sage environment variables set.  Don't forget
to exit when you are done.  Beware:
 * Do not do anything with other copies of Sage on your system.
 * Do not use this for installing Sage packages using "sage -i" or for
   running "make" at Sage's root directory.  These should be done
   outside the Sage shell.

Bypassing shell configuration files...

Note: SAGE_ROOT=/home/kwankyu/GitHub/sage-dev
(sage-sh) kwankyu@HOME:sage-dev$ info --version
info (GNU texinfo) 7.1

Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
(sage-sh) kwankyu@HOME:sage-dev$ which info
/usr/bin/info

I am testing on Ubuntu on WSL

There is nothing changed with your updated branch.

I will force sage to build the info spkg...

kwankyu · 2024-11-07T22:11:32Z

sage: !info --version
info (GNU texinfo) 7.0.3

Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
sage: from sage.interfaces.singular import get_docstring
sage: get_docstring('Preface')
'\n1 Preface\n*********\n\n                        SINGULAR version 4.4.0\n                  University of Kaiserslautern-Landau\n      Department of Mathematics and  Centre for Computer Algebra\n     Authors: W. Decker, G.-M. Greuel, G. Pfister, H. Schoenemann\n\n                        Copyright (C) 1986-2024\n\n\n                               *NOTICE*\n\nThis program is free software; you can redistribute it and/or modify it\nunder the terms of the GNU General Public License as published by the\nFree Software Foundation (version 2 or version 3 of the License).\n\nSome single files have a copyright given within the file:\nSingular/links/ndbm.* (BSD)\n\nThe following software modules shipped with SINGULAR have their own\ncopyright: the omalloc library, the readline library, the GNU Multiple\nPrecision Library (GMP), NTL: A Library for doing Number Theory (NTL),\nFlint: Fast Library for Number Theory, the Singular-Factory library, the\nSingular-Factory library, the Singular-libfac library, surfex, and, for\nthe Windows distributions, the Cygwin DLL and the Cygwin tools\n(Cygwin), and the XEmacs editor (XEmacs).\n\nTheir copyrights and licenses can be found in the accompanying files\nCOPYING which are distributed along with these packages.  (Since\nversion 3-0-3 of SINGULAR, al
...
sage: singular.aliggggg?
Signature:      singular.aliggggg(*args, **kwds)
Type:           SingularFunction
String form:    aliggggg
File:           ~/GitHub/sage-dev/src/sage/interfaces/singular.py
Docstring:
This function is an automatically generated pexpect wrapper around the
Singular function 'aliggggg'.

EXAMPLES:

   sage: groebner = singular.groebner
   sage: P.<x, y> = PolynomialRing(QQ)
   sage: I = P.ideal(x^2-y, y+x)
   sage: groebner(singular(I))
   x+y,
   y^2-y
Init docstring:
WARNING: the enclosing module is marked 'needs sage.libs.gap sage.libs.pari sage.libs.singular sage.symbolic',
so doctests may not pass.
Call docstring:
WARNING: the enclosing module is marked 'needs sage.libs.gap sage.libs.pari sage.libs.singular sage.symbolic',
so doctests may not pass.

Nothing changed....

orlitzky · 2024-11-07T23:10:59Z

Argh, OK, I see the problem. When I updated the PR to trust the exit code from info, that accidentally allowed it to match things like "Preface" again. I'll take another shot at it tomorrow, thank you.

orlitzky · 2024-11-08T14:10:56Z

I've added back the check for the section header to ensure that get_docstring("Preface") fails, and I've added a doctest for that now.

I also tried to fix the singular.nonexistent? problem. One of the functions in sage.misc.sageinspect was ignoring all exceptions that arise inside the question-mark help. I think the motivation for that is obsolete, so I've removed it, but we'll have to wait for the documentation build to complete to be sure.

In any case, singular.nonexistent? now raises an error immediately instead of showing you junk. Likewise, running

sage: from sage.libs.singular.function import singular_function
sage: f = singular_function("nonexistent")
sage: f?

should raise an error.

Before we can replace our hand-rolled parser for Singular's info file, we need to be able to detect the "info" program from the GNU Info package. The goal is to politely disable access to Singular's info if GNU Info is unavailable. This avoids a hard dependency on GNU Info in the sage library. We require v7 or later because a bugfix in Texinfo v7.0.0 corrects the exit code from "info" when the requested node is not found. This allows us to separate the "expected failures" from the truly unexpected ones.

Our Singular interface currently contains a hand-written parser for Singular's "info" file. This commit eliminates the custom parser in favor of launching GNU Info. GNU Info (or its superset, Texinfo) are widespread, portable, and easy to install on all of the systems we support, so in most cases this should be a "free" improvement. The hand-written parser has several drawbacks: * The extra code is a maintenance burden. We should not be wasting our time reimplementing standard tools. * The custom parser is buggy. For example, it is supposed to raise a KeyError when documentation for a non-existent function is requested. However, the parser does not keep track of what section it's in, so, for example, get_docstring("Preface") returns the contents of the preface even though "Preface" is not a Singular function. * The first time documentation is requested, the entire info file is loaded into a dictionary. This wastes a few megabytes of memory for the duration of the Sage session. * The custom parser does not handle compression (GNU Info does transparently), and the end user or people packaging Singular may not be aware of that. If the system installation of Singular has a compressed info file, Sage won't be able to read it. For contrast, the one downside to using GNU Info is that it adds a new runtime dependency to sagelib. To mitigate that, we do not technically require it, and instead raise a warning if the user (a) tries to read the Singular documentation and (b) has managed to find a system without GNU Info. Our singular_console() itself tries to launch GNU Info to display its interactive help, so the additional optional dependency is not so additional except in corner cases, such as a pypi installation of a subset of Sage linked against libsingular but without a full Singular install.

The rewritten get_docstring() in sage.interfaces.singular supports formatting the output as reStructuredText, so we no longer need to do it "by hand" in the library interface.

…atted Our _sage_getdoc_unformatted() catches all exceptions and turns them into the empty string. The side effects of this are pretty bad: this function is used to generate the help? strings in the sage REPL, and if something goes wrong, the user will get a confusing blank docstring rather than the error. The change was introduced in issue 19671 as a workaround for a runtime error raised inside flask. But we are no longer using flask, and hopefully no other parts of the Sage library raise an exception during the documentation build. We are about to find out!

These modules use singular_function() from sage.libs.singular.function to create top-level aliases for a few singular functions. The docbuild however tries to build documentation for these functions, and it can raise an error if GNU info is not installed. There's no good reason to define them globally in the first place (they are only used locally), so an easy fix is to move the imports and variables local.

orlitzky · 2024-11-09T03:36:12Z

It's looking like it might be OK now:

The section header parsing that was required with GNU info < 7.0 is back because that's what allows get_docstring("Preface") to be caught and fail.
As a result, there's no point in requiring info v7. It will accept v6 again.
I removed the exception masking in sage's help?. This fixes the docs for nonexistent singular functions (they now raise an error).
I had to make a few singular function variables in src.sage.algebras.letterplace local so that the docbuild wouldn't try to document them, but afterwards there doesn't seem to be any problem building the docs.
I rebased and squashed everything because there's a new develop branch and I had to force push anyway.

src/sage/interfaces/singular.py

kwankyu · 2024-11-09T05:30:36Z

Thanks. It now works nicely, both with spkg info (7.0.3) and system info (7.1).

One anomaly I noticed: for this input

sage: singular.ali?

I get

Signature:      singular.ali(*args, **kwds)
Type:           SingularFunction
String form:    ali
File:           ~/GitHub/sage-dev/src/sage/interfaces/singular.py
Docstring:
This function is an automatically generated pexpect wrapper around the
Singular function 'ali'.

EXAMPLES:

   sage: groebner = singular.groebner
   sage: P.<x, y> = PolynomialRing(QQ)
   sage: I = P.ideal(x^2-y, y+x)
   sage: groebner(singular(I))
   x+y,
   y^2-y

The Singular documentation for "ali" is given below.

   5.1.1 align
   -----------

   `*Syntax:*'
        `align (' vector_expression, int_expression `)'
        `align (' module_expression, int_expression `)'

   `*Type:*'
        type of the first argument

Completing "ali" to "align" is nice, but I think the documentation should show "align" instead of "ali".

But to be fair, this anomaly is not new. Let me know if you would fix it or leave it.

kwankyu · 2024-11-09T05:41:46Z

Some strange thing is happening in documentation:

https://doc-pr-38899--sagemath.netlify.app/html/en/reference/interfaces/sage/interfaces/singular#hunk2

orlitzky · 2024-11-09T14:47:27Z

Completing "ali" to "align" is nice, but I think the documentation should show "align" instead of "ali".

But to be fair, this anomaly is not new. Let me know if you would fix it or leave it.

I don't think this will be easy to fix. The problem is that singular.ali is a "valid" singular function object created on-the-fly by the pexpect interface. You can do the same thing with e.g. gap.ali?. It will create a GapFunction called gap.ali, and then return whatever the docstring is for it.

The thing that I could fix here is that ali? will load the help for align and not insist on an exact match for ali. But maybe it is nicer to return the nearest match, I don't know. It would be great if singular.ali<tab> worked too but for some reason "align" isn't in the list of tab completions.

orlitzky · 2024-11-09T15:11:23Z

This is probably something a singular expert should look in to, but I think the names of the built-in singular functions can be obtained from the list returned by reservedName(); after filtering out the data types and other non-functions.

It looks like the OUTPUT block for singular_version() was copy/pasted from somewhere else -- it doesn't make sense. We delete it; the remaining short description of this function is sufficient.

orlitzky · 2024-11-09T15:16:55Z

Some strange thing is happening in documentation:

That was literally the docstring for singular_version(). Probably a copy/paste mistake. I've fixed it.

kwankyu · 2024-11-10T05:25:06Z

Completing "ali" to "align" is nice, but I think the documentation should show "align" instead of "ali".
But to be fair, this anomaly is not new. Let me know if you would fix it or leave it.

I don't think this will be easy to fix.

OK.

kwankyu

LGTM now. Thanks.

kwankyu · 2024-11-10T05:29:31Z

Would you add "Fixes #32242" in the PR description?

orlitzky · 2024-11-10T13:57:46Z

Done, thank you for the careful review

…ad Singular's info Our Singular interface currently contains a hand-written parser for Singular's "info" file. This commit eliminates the custom parser in favor of launching GNU Info. GNU Info (or its superset, Texinfo) are widespread, portable, and easy to install on all of the systems we support, so in most cases this should be a "free" improvement. The hand-written parser has several drawbacks: * The extra code is a maintenance burden. We should not be wasting our time reimplementing standard tools. * The custom parser is buggy. For example, it is supposed to raise a KeyError when documentation for a non-existent function is requested. However, the parser does not keep track of what section it's in, so, for example, get_docstring("Preface") returns the contents of the preface even though "Preface" is not a Singular function. * The first time documentation is requested, the entire info file is loaded into a dictionary. This wastes a few megabytes of memory for the duration of the Sage session. * The custom parser does not handle compression (GNU Info does transparently), and the end user or people packaging Singular may not be aware of that. If the system installation of Singular has a compressed info file, Sage won't be able to read it. For contrast, the one downside to using GNU Info is that it adds a new runtime dependency to sagelib. To mitigate that, we do not technically require it, and instead raise a warning if the user (a) tries to read the Singular documentation and (b) has managed to find a system without GNU Info. Our singular_console() itself tries to launch GNU Info to display its interactive help, so the additional optional dependency is not so additional except in corner cases, such as a pypi installation of a subset of Sage linked against libsingular but without a full Singular install. This was originally sagemath#32242 Since then, * GNU Info has become a standard package in the sage distribution * I'm now using `sage.features.info` to detect the "info" program, so nothing bad happens if it's not installed Fixes sagemath#32242 URL: sagemath#38899 Reported by: Michael Orlitzky Reviewer(s): Kwankyu Lee, Michael Orlitzky

github-actions bot added the s: needs review label Oct 31, 2024

orlitzky marked this pull request as draft October 31, 2024 23:02

github-actions bot removed the s: needs review label Oct 31, 2024

orlitzky force-pushed the use-gnu-info-to-parse-singulars-info branch 2 times, most recently from 4314aa8 to 69e8efd Compare November 1, 2024 03:35

orlitzky added the s: needs review label Nov 1, 2024

kwankyu marked this pull request as ready for review November 2, 2024 07:59

kwankyu reviewed Nov 7, 2024

View reviewed changes

src/sage/interfaces/singular.py Show resolved Hide resolved

orlitzky force-pushed the use-gnu-info-to-parse-singulars-info branch 2 times, most recently from d07e649 to 4e79070 Compare November 8, 2024 14:04

orlitzky added 4 commits November 8, 2024 19:43

src/sage/libs/singular/function.pyx: use new get_docstring() features

1045e49

The rewritten get_docstring() in sage.interfaces.singular supports formatting the output as reStructuredText, so we no longer need to do it "by hand" in the library interface.

orlitzky force-pushed the use-gnu-info-to-parse-singulars-info branch from 4e79070 to d52b5e5 Compare November 9, 2024 01:23

orlitzky force-pushed the use-gnu-info-to-parse-singulars-info branch from d52b5e5 to 5f8f525 Compare November 9, 2024 02:04

kwankyu reviewed Nov 9, 2024

View reviewed changes

src/sage/interfaces/singular.py Outdated Show resolved Hide resolved

src/sage/interfaces/singular.py: False -> False in documentation

fbd173a

src/sage/interfaces/singular.py: remove nonsense OUTPUT block

0275387

It looks like the OUTPUT block for singular_version() was copy/pasted from somewhere else -- it doesn't make sense. We delete it; the remaining short description of this function is sufficient.

kwankyu approved these changes Nov 10, 2024

View reviewed changes

kwankyu added s: positive review and removed s: needs review labels Nov 10, 2024

kwankyu added the c: interfaces label Nov 10, 2024

vbraun merged commit a9f3f77 into sagemath:develop Nov 16, 2024
22 of 24 checks passed

github-actions bot removed the s: positive review label Nov 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src/sage/interfaces/singular.py: use GNU Info to read Singular's info #38899

src/sage/interfaces/singular.py: use GNU Info to read Singular's info #38899

orlitzky commented Oct 31, 2024 •

edited

Loading

github-actions bot commented Oct 31, 2024 •

edited

Loading

kwankyu commented Nov 7, 2024

kwankyu commented Nov 7, 2024

kwankyu commented Nov 7, 2024

kwankyu commented Nov 7, 2024

orlitzky commented Nov 7, 2024

orlitzky commented Nov 7, 2024

orlitzky commented Nov 7, 2024

kwankyu commented Nov 7, 2024

kwankyu commented Nov 7, 2024

orlitzky commented Nov 7, 2024

orlitzky commented Nov 8, 2024

orlitzky commented Nov 9, 2024

kwankyu commented Nov 9, 2024 •

edited

Loading

kwankyu commented Nov 9, 2024

orlitzky commented Nov 9, 2024

orlitzky commented Nov 9, 2024

orlitzky commented Nov 9, 2024

kwankyu commented Nov 10, 2024

kwankyu left a comment

kwankyu commented Nov 10, 2024

orlitzky commented Nov 10, 2024

src/sage/interfaces/singular.py: use GNU Info to read Singular's info #38899

src/sage/interfaces/singular.py: use GNU Info to read Singular's info #38899

Conversation

orlitzky commented Oct 31, 2024 • edited Loading

github-actions bot commented Oct 31, 2024 • edited Loading

kwankyu commented Nov 7, 2024

kwankyu commented Nov 7, 2024

kwankyu commented Nov 7, 2024

kwankyu commented Nov 7, 2024

orlitzky commented Nov 7, 2024

orlitzky commented Nov 7, 2024

orlitzky commented Nov 7, 2024

kwankyu commented Nov 7, 2024

kwankyu commented Nov 7, 2024

orlitzky commented Nov 7, 2024

orlitzky commented Nov 8, 2024

orlitzky commented Nov 9, 2024

kwankyu commented Nov 9, 2024 • edited Loading

kwankyu commented Nov 9, 2024

orlitzky commented Nov 9, 2024

orlitzky commented Nov 9, 2024

orlitzky commented Nov 9, 2024

kwankyu commented Nov 10, 2024

kwankyu left a comment

Choose a reason for hiding this comment

kwankyu commented Nov 10, 2024

orlitzky commented Nov 10, 2024

orlitzky commented Oct 31, 2024 •

edited

Loading

github-actions bot commented Oct 31, 2024 •

edited

Loading

kwankyu commented Nov 9, 2024 •

edited

Loading