Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

singular.version() has no doctest #5994

Closed
simon-king-jena opened this issue May 6, 2009 · 62 comments
Closed

singular.version() has no doctest #5994

simon-king-jena opened this issue May 6, 2009 · 62 comments

Comments

@simon-king-jena
Copy link
Member

Neither singular.version nor singular_version have doc tests.

Upstream: Completely fixed; Fix reported upstream

CC: @kedlaya

Component: interfaces

Keywords: singular version help.cnf

Author: Jori Mäntysalo

Branch/Commit: 87c73ac

Reviewer: Jeroen Demeyer

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

@simon-king-jena
Copy link
Member Author

comment:1

Remark:

Apparently the problem is on the side of Singular.

In Singular, freshly started, when you do

> system("--version");

there is the error (it says "? cannot open help.cnf").
If you repeat, no error.

If people think that my suggestion would break code, I suggest to have a new method singular.version_number().

@simon-king-jena
Copy link
Member Author

comment:2

Replying to @simon-king-jena:

Apparently the problem is on the side of Singular.

And meanwhile it is reported upstream.

I do not know, however, if the problem still occurs with Singular 3-1-0 (I only have a beta version, and this still shows the error).

@simon-king-jena
Copy link
Member Author

comment:3

Replying to @simon-king-jena:

Replying to @simon-king-jena:
And meanwhile it is reported upstream.

I do not know, however, if the problem still occurs with Singular 3-1-0 (I only have a beta version, and this still shows the error).

"Upstream" answered, and it seems that the problem is fixed in the official release.

I could verify that on the server in Oberwolfach, the error does not occur in Singular 3-1-0, but it does occur in Oberwolfach in Singular 3-0-4 (as being part of Sage). Opinions?

But there still remains the question if singular.version() ought to return a tuple of integers, rather than a lengthy string with all build informations.

@aghitza
Copy link

aghitza commented May 6, 2009

comment:4

Replying to @simon-king-jena:

"Upstream" answered, and it seems that the problem is fixed in the official release.

I could verify that on the server in Oberwolfach, the error does not occur in Singular 3-1-0, but it does occur in Oberwolfach in Singular 3-0-4 (as being part of Sage). Opinions?

We can probably wait until after Sage Days 15 for this?

But there still remains the question if singular.version() ought to return a tuple of integers, rather than a lengthy string with all build informations.

My personal preference would be for it to return a tuple of integers, but we could give it an optional argument with_build_info=True (or verbatim=True or something similar) to make it return the lengthy string.

@simon-king-jena
Copy link
Member Author

comment:5

Replying to @simon-king-jena:

I could verify that on the server in Oberwolfach, the error does not occur in Singular 3-1-0, but it does occur in Oberwolfach in Singular 3-0-4 (as being part of Sage). Opinions?

The "Opinions?" bit should be two lines lower. Sorry.

It turned out that the error occurs in any Sage-built Singular version that I know: Singular 3-0-4 on sage.math, on two of my computers, in Oberwolfach, and Singular 3-1-0-Beta on sage.math and on one of my computers.

If Singular is not built by Sage, then the error apparently does not occur: With Singular 3-0-3 (very old) on one of my machines, Singular-3-1-0-Beta on my machine (same sources, modulo the usual patches, as the sage-built version!), and Singular-3-1-0 official release in Oberwolfach.

So, after all, it seems to me that it is all Sage's fault.

But there still remains the question if singular.version() ought to return a tuple of integers, rather than a lengthy string with all build informations.

Here is where I want to know some "Opinions"...

@williamstein
Copy link
Contributor

comment:6

The attached patch fixes the output format for version to be more consistent with the other interfaces, e.g., gp.version(). It also programs around the issue with help files, which is not fixed in Singular-3-1-0... and I'm not at all clear why it is considered a bug in Singular by the people in the thread above.

@simon-king-jena
Copy link
Member Author

comment:7

Attachment: trac_5994.patch.gz

Replying to @williamstein:

It also programs around the issue with help files, which is not fixed in Singular-3-1-0... and I'm not at all clear why it is considered a bug in Singular by the people in the thread above.

Note that I originally thought that the issue with help files is a problem of Singular, and clearly a bug: I mean, you ask for the version number and get an error; you ask again, and it works! It had reported it upstream.

But then the impression came across (see my last post) that it only occurs in Singular if it is built by Sage. In this case, it could be a problem with the patched version in Sage, which might be worth another ticket.

Cheers,
Simon

@williamstein
Copy link
Contributor

comment:8

I think the first time you ask for the version, singular fires up its help system, and reports a bug about it not being properly configured by us for such use. Then it doesn't report that again, since it already did. I think it is very sensible behavior by Singular.

So can you review this?

@wjp
Copy link
Mannequin

wjp mannequin commented Jan 19, 2010

comment:9

For the record, adding an empty $SAGE_ROOT/local/share/singular/help.cnf suppresses the error. There is actually a help.cnf file in the singular sources (, but it doesn't seem to get installed. Its contents don't look to be particularly relevant for sage at first glance, though.

@simon-king-jena
Copy link
Member Author

comment:10

Replying to @williamstein:

So can you review this?

I'd like to, but the sage-4.3.1.alpha1 that I had built on sage-math seems broken. It used to work, but when I did ./sage -br main, it failed with

ImportError: /usr/lib/libstdc++.so.6: version `GLIBCXX_3.4.11' not found (required by /home/SimonKing/SAGE/sage-4.3.1.alpha1/local/lib/libgmpxx.so.3)

Might ./sage -ba work?

Anyway, I'd like to see one more doc test, that checks consistency with another way of getting the version number -- just for consistency:

sage: tuple([Integer(c) for c in singular.eval('system("version")')][:3] == singular.version()[0]
True

@simon-king-jena
Copy link
Member Author

comment:11

Replying to @simon-king-jena:

...

sage: tuple([Integer(c) for c in singular.eval('system("version")')][:3] == singular.version()[0]
True

Oops, apparently I forgot a closing bracket on the left hand side. Anyway, you know what this prospective doc-test is supposed to do...

@simon-king-jena
Copy link
Member Author

comment:12

Replying to @simon-king-jena:

Might ./sage -ba work?

It did not work. I fear that I have to start from scratch, so that it will take some hours before I will be able to review the patch.

@simon-king-jena
Copy link
Member Author

comment:13

OK, meanwhile I built sage-4.3.1.rc1

The patch applies cleanly.

However, I don't like the doc tests, and I think the return value is wrong.

The tests check that the first version number is 3. OK, it will eventually change, but not in the near future.

Then they test that the version number is of length 3. Can we rely on it? There used to be two-digit versions.

In fact, the "official" version number seems to be four digits, not three:

sage: singular.eval('system("version")')
'3104'

Hence, the first return value of singular.version() should be (3,1,0,4) not (3,1,0).
Note that according to the Singular homepage there is a version 3-1-0-6 available, so, the last digit does play a role.

So, my questions are:

  • How important is it to have doc tests that remain valid if the Singular version changes?
  • Do we take the patch level version number of Singular serious?

@simon-king-jena
Copy link
Member Author

comment:14

Replying to @simon-king-jena:

So, my questions are:

  • How important is it to have doc tests that remain valid if the Singular version changes?
  • Do we take the patch level version number of Singular serious?

Is there an answer, yet?

@simon-king-jena
Copy link
Member Author

comment:15

-- bump --

First question: Do we want that singular.version() works? Currently, it fails when first called.

Second question: Do we want that (at least by default) it returns a tuple of three or four numbers (three- resp four-digit vesion numbers), or do people like that the output of singular.version() (if it is called again after the initial error) returns a lengthy string with full information on the way Singular has been built?

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 7, 2011

comment:16

It shouldn't hurt to install the help.cnf file though, in which case we wouldn't need (and should remove) the try ... except RuntimeError ... and a second try.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 7, 2011

Changed keywords from singular version to singular version help.cnf

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 7, 2011

Author: William Stein

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 7, 2011

comment:17

Since he reported this again on a duplicate, #11519.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin removed this from the sage-6.1 milestone Jan 30, 2014
@simon-king-jena
Copy link
Member Author

comment:43

Upstream says that help.cnf should indeed be put into the same folder as all the other library files, i.e., local/share/singular/.

@simon-king-jena
Copy link
Member Author

Changed upstream from Reported upstream. No feedback yet. to Reported upstream. Developers deny it's a bug.

@jdemeyer
Copy link

comment:44

Replying to @simon-king-jena:

Replying to @jdemeyer:

Replying to @simon-king-jena:

But for now, we need a work-around in Sage.

I suggest you just change spkg-install to touch the empty file $SAGE_SHARE/singular/help.cnf

Why not modify spkg-install, so that it installs the missing file? Alternatively, we could patch Singular's Makefile (or whatever it is called) so that it installs the missing file. What is the preferred way?

I don't really care much.

Hang on. I just notice that the singular spkg apparently screws completely. Or how would you explain that I find the complete UNPACKED singular source tree in SAGE_LOCAL/upstream/ ?

Probably you once extracted the tarball in upstream? It's not the package which does that...

@simon-king-jena
Copy link
Member Author

comment:45

Replying to @jdemeyer:

Why not modify spkg-install, so that it installs the missing file? Alternatively, we could patch Singular's Makefile (or whatever it is called) so that it installs the missing file. What is the preferred way?

I don't really care much.

Then I guess we should do it in spkg-install.

Probably you once extracted the tarball in upstream? It's not the package which does that...

Could be. I just deleted it and did "sage -f singular".

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 10, 2015

comment:46

There's also #sagemath on freenode. Just saying...

@simon-king-jena
Copy link
Member Author

comment:47

Replying to @nexttime:

There's also #sagemath on freenode. Just saying...

Meaning what?

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 10, 2015

comment:48

Replying to @simon-king-jena:

Replying to @nexttime:

There's also #sagemath on freenode. Just saying...

Meaning what?

Overfull inbox, TeX would have said. ;-)

@simon-king-jena
Copy link
Member Author

comment:49

It seems the current agreement seems to be to modify spkg-install, so that it copies help.cnf from the singular sources to $SAGE_SHARE/singular/. What is the path to the singular sources while spkg-install is executed?

@jdemeyer
Copy link

comment:50

Replying to @simon-king-jena:

What is the path to the singular sources while spkg-install is executed?

It's the current working directory.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 24, 2016

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 24, 2016

Changed commit from a2ae8c8 to 87c73ac

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 24, 2016

comment:52

Error has gone away with Singular 4.

I added doctest.

What goes to output format, I think this should be changed in all *_version() commands at once. (Assuming somebody would care, which is propably not true.)


New commits:

87c73acDoctest singular_version().

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 24, 2016

Changed author from William Stein, Simon King to Jori Mäntysalo

@jm58660

This comment has been minimized.

@jm58660
Copy link
Mannequin

jm58660 mannequin commented Oct 24, 2016

Changed upstream from Reported upstream. Developers deny it's a bug. to Completely fixed; Fix reported upstream

@jm58660 jm58660 mannequin added p: minor / 4 and removed p: major / 3 labels Oct 24, 2016
@jm58660 jm58660 mannequin modified the milestones: sage-6.4, sage-7.5 Oct 24, 2016
@jm58660 jm58660 mannequin added s: needs review and removed s: needs work labels Oct 24, 2016
@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title singular.version() yields an error when first called, has no doctest, and has a strange output imo singular.version() has no doctest Oct 26, 2016
@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Nov 2, 2016

Changed branch from u/jmantysalo/singular_version_yielding_error to 87c73ac

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants