Posets constructor does not check argument

[jm58660]

sage: P=Posets("cat-says-meow"); P
Posets containing cat-says-meow vertices
sage: P[42]
?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~

CC: @nathanncohen

Component: combinatorics

Author: Jori Mäntysalo

Branch/Commit: a37a626

Reviewer: Nathann Cohen

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

[jm58660]

comment:1

Is there some reason for not to check arguments on other functions? For example

Posets.RestrictedIntegerPartitions('cat-says-meow')
Posets.SymmetricGroupBruhatIntervalPoset('cat-says-meow', 42)
Posets.BooleanLattice('cat-says-meow')

will all give exception, but they output

ValueError: n must be an integer or be equal to one of None, NN,
NonNegativeIntegers()

ValueError: invalid literal for int() with base 10: ''

TypeError: unsupported operand type(s) for ** or pow(): 'int' and 'str'

Is right check just if not n in ZZ: raise...?

[nathanncohen]

comment:2

Well, as long as an exception is raised I do not mind much, personally. Though you can add an explicit check if it helps you sleep better :-P

Nathann

[jm58660]

comment:3

BooleanLattice(1) return 1-element lattice, not 2-element. Hence I raise this from trivial level to minor. (And write this comment as a note to myself.)

[jm58660]

Branch: u/jmantysalo/posets_constructor_does_not_check_argument

[jm58660]

comment:5

I copied the check from RandomPoset.

---

New commits:

335c546

Correction for BooleanLattice(n) for n=0 and n=1; some checks for input.

[jm58660]

Commit: 335c546

[jm58660]

Author: Jori Mäntysalo

[nathanncohen]

comment:7

Hello!

It looks good but you should check the doctests. There are errors when you run them in the poset folder.

Nathann

[jm58660]

comment:8

Replying to @nathanncohen:

It looks good but you should check the doctests. There are errors when you run them in the poset folder.

I do not understand. It says Got: <BLANKLINE>. Only change I did was changing "numbers" to "Numbers" in phrase "number of elements must be non-negative - -" at RandomPoset(). Of course I should have changed doctest also. But where does this blankline comes?

[nathanncohen]

comment:9

Yo !

I do not understand. It says Got: <BLANKLINE>. Only change I did was changing "numbers" to "Numbers" in phrase "number of elements must be non-negative - -" at RandomPoset(). Of course I should have changed doctest also. But where does this blankline comes?

The blankline is a bluff. You did not only change the n->N but also the final '.'.

Sphinx is not the kind of software that you can trust to give meaningful error messages ^^;

Nathann

[sagetrac-git]

Changed commit from 335c546 to ef5cc86

[sagetrac-git]

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

ef5cc86

Docstring corrected.

[jm58660]

comment:11

Replying to @nathanncohen:

Sphinx is not the kind of software that you can trust to give meaningful error messages ^^;

Grrrr....

Thanks, corrected this, also added another dots.

[nathanncohen]

comment:12

Thanks, corrected this, also added another dots.

Okay ! I will run all of Sage's doctests because the Poset code is called in weird places, then will switch change the ticket's status.

Nathann

[nathanncohen]

comment:13

Done, and no problem !

Nathann

[nathanncohen]

Reviewer: Nathann Cohen

[vbraun]

comment:14

Python style for exeption messages is incomplete sentences. In particular, no capitalization of the first word and no period at the end. E.g.

$ python
Python 2.7.8 (default, Oct 27 2014, 15:54:28) 
[GCC 4.9.1 20140930 (Red Hat 4.9.1-11)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> 1 + '2'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for +: 'int' and 'str'

This ticket makes it worse in many places.

[sagetrac-git]

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

a37a626

Changed exception messages to incomplete sentences.

[sagetrac-git]

Changed commit from ef5cc86 to a37a626

[jm58660]

comment:16

Replying to @vbraun:

Python style for exeption messages is incomplete sentences.

OK. Corrected.

[nathanncohen]

comment:17

Tests pass !

(sorry for the incompatible writing styles...)

Nathann

[vbraun]

Changed branch from u/jmantysalo/posets_constructor_does_not_check_argument to a37a626