Sage: Ticket #18159: cardinality must output Infinty or a Sage integer
https://trac.sagemath.org/ticket/18159
<p>
We add a <code>._test_cardinality()</code> method in the category <code>Sets</code> that takes care of checking that the output of <code>.cardinality()</code> is a Sage integer or +Infinity.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/18159
Trac 1.1.6vdelecroixSat, 11 Apr 2015 14:48:01 GMTcommit, branch, author set
https://trac.sagemath.org/ticket/18159#comment:1
https://trac.sagemath.org/ticket/18159#comment:1
<ul>
<li><strong>commit</strong>
set to <em>28378f014398fbb4f94836a47e3098210c8e4588</em>
</li>
<li><strong>branch</strong>
set to <em>u/vdelecroix/18159</em>
</li>
<li><strong>author</strong>
set to <em>Vincent Delecroix</em>
</li>
</ul>
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=28378f014398fbb4f94836a47e3098210c8e4588"><span class="icon"></span>28378f0</a></td><td><code>Trac 18159: add a ._test_cardinality in Sets category</code>
</td></tr></table>
TicketgitSat, 11 Apr 2015 16:09:27 GMTcommit changed
https://trac.sagemath.org/ticket/18159#comment:2
https://trac.sagemath.org/ticket/18159#comment:2
<ul>
<li><strong>commit</strong>
changed from <em>28378f014398fbb4f94836a47e3098210c8e4588</em> to <em>17924b8da8777805ca17289e00e62c3beca5a146</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=17924b8da8777805ca17289e00e62c3beca5a146"><span class="icon"></span>17924b8</a></td><td><code>Trac 18159: fix cardinality/is_finite in sets/set.py</code>
</td></tr></table>
TicketncohenSat, 11 Apr 2015 16:27:07 GMT
https://trac.sagemath.org/ticket/18159#comment:3
https://trac.sagemath.org/ticket/18159#comment:3
<p>
What is wrong with an 'int'? <code>O_o</code>
</p>
TicketvdelecroixSat, 11 Apr 2015 16:51:38 GMT
https://trac.sagemath.org/ticket/18159#comment:4
https://trac.sagemath.org/ticket/18159#comment:4
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18159#comment:3" title="Comment 3">ncohen</a>:
</p>
<blockquote class="citation">
<p>
What is wrong with an 'int'? <code>O_o</code>
</p>
</blockquote>
<p>
<code>2/3</code> for example.
</p>
TicketncohenSat, 11 Apr 2015 16:53:53 GMT
https://trac.sagemath.org/ticket/18159#comment:5
https://trac.sagemath.org/ticket/18159#comment:5
<p>
I do not understand. It seems that your code detects it as 'wrong' if a function <code>.cardinality()</code> returns <code>int(4)</code>. Why is that?
</p>
TicketvdelecroixSat, 11 Apr 2015 16:54:58 GMTdescription changed
https://trac.sagemath.org/ticket/18159#comment:6
https://trac.sagemath.org/ticket/18159#comment:6
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/18159?action=diff&version=6">diff</a>)
</li>
</ul>
TicketvdelecroixSat, 11 Apr 2015 16:59:25 GMT
https://trac.sagemath.org/ticket/18159#comment:7
https://trac.sagemath.org/ticket/18159#comment:7
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18159#comment:5" title="Comment 5">ncohen</a>:
</p>
<blockquote class="citation">
<p>
I do not understand. It seems that your code detects it as 'wrong' if a function <code>.cardinality()</code> returns <code>int(4)</code>. Why is that?
</p>
</blockquote>
<p>
Yes. And the reason is because Python int do not behave as Sage integers. For example
</p>
<pre class="wiki">sage: S = my_finite_set()
sage: S.cardinality().factor()
sage: ~(S.cardinality())
</pre><p>
For coherence, we need to have some specifications for <code>.cardinality</code>.
</p>
<p>
The main reason for that ticket was that some sets were actually returning rational numbers see <a class="ext-link" href="http://trac.sagemath.org/ticket/17852#comment:92"><span class="icon"></span>http://trac.sagemath.org/ticket/17852#comment:92</a>
</p>
TicketncohenSat, 11 Apr 2015 17:03:02 GMT
https://trac.sagemath.org/ticket/18159#comment:8
https://trac.sagemath.org/ticket/18159#comment:8
<p>
That they behave differently is not (to me) a sufficient reason to prevent everybody from returning python ints. Checking that the output can be turned into an int would be sufficient to detect this problem with rational numbers.
</p>
TicketvdelecroixSat, 11 Apr 2015 17:06:03 GMT
https://trac.sagemath.org/ticket/18159#comment:9
https://trac.sagemath.org/ticket/18159#comment:9
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18159#comment:8" title="Comment 8">ncohen</a>:
</p>
<blockquote class="citation">
<p>
That they behave differently is not (to me) a sufficient reason to prevent everybody from returning python ints. Checking that the output can be turned into an int would be sufficient to detect this problem with rational numbers.
</p>
</blockquote>
<p>
All right, output string is accepted as well?
</p>
<p>
This is very bad. Imagine I need something like:
</p>
<pre class="wiki">sage: vector([s.cardinality() for s in S])
</pre><p>
If any of the <code>s.cardinality()</code> returns a rational, I end up with a vector over rational numbers (which are much slower than over integers).
</p>
<p>
It is very similar to <code>is_X</code> functions that return boolean answers. You might complain if they start returning <code>gap</code> booleans or <code>numpy</code> boolean objects.
</p>
<p>
Vincent
</p>
TicketncohenSat, 11 Apr 2015 17:12:29 GMT
https://trac.sagemath.org/ticket/18159#comment:10
https://trac.sagemath.org/ticket/18159#comment:10
<p>
What would you have against testing that it is an int, a long, an Integer, or equal to (not 'is') infinity?
</p>
<p>
Nathann
</p>
TicketvdelecroixSat, 11 Apr 2015 17:21:44 GMT
https://trac.sagemath.org/ticket/18159#comment:11
https://trac.sagemath.org/ticket/18159#comment:11
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18159#comment:10" title="Comment 10">ncohen</a>:
</p>
<blockquote class="citation">
<p>
What would you have against testing that it is an int, a long, an Integer, or equal to (not 'is') infinity?
</p>
</blockquote>
<p>
You do not want to see a Sage rational anymore? And what about <code>numpy</code> integers, <code>pari</code> integers, <code>gp</code> integers, <code>gap</code> integers and <code>libgap</code> integers?
</p>
<p>
Testing equality to infinity might be broken or you can have objects that want to be equal to everybody (why not?).
</p>
<p>
The specification in <code>sage.categories.sets_cat</code> is:
</p>
<pre class="wiki">``self.cardinality()`` should return the cardinality of the set
``self`` as a sage :class:`Integer` or as ``infinity``.
</pre><p>
If you complain that it should not be the case then you should argue why. And not telling me to argue why not.
</p>
<p>
Vincent
</p>
TicketncohenSat, 11 Apr 2015 17:26:10 GMT
https://trac.sagemath.org/ticket/18159#comment:12
https://trac.sagemath.org/ticket/18159#comment:12
<p>
I think that it should not be the case because python ints/long are smaller and faster than Sage Integers, and that we should not be forced to instantiate Integers if we do not need to.
</p>
<p>
For the very same reason you invoked, earlier, when you said that you did not want rationals because of the possible loss of time.
</p>
<p>
Nathann
</p>
TicketncohenSat, 11 Apr 2015 17:31:27 GMT
https://trac.sagemath.org/ticket/18159#comment:13
https://trac.sagemath.org/ticket/18159#comment:13
<p>
(note that your problems are solved if you specify ZZ when you build your vector, isn't it?)
</p>
TicketvdelecroixSat, 11 Apr 2015 17:32:49 GMTdescription changed
https://trac.sagemath.org/ticket/18159#comment:14
https://trac.sagemath.org/ticket/18159#comment:14
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/18159?action=diff&version=14">diff</a>)
</li>
</ul>
TicketvdelecroixSat, 11 Apr 2015 17:37:48 GMT
https://trac.sagemath.org/ticket/18159#comment:15
https://trac.sagemath.org/ticket/18159#comment:15
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18159#comment:12" title="Comment 12">ncohen</a>:
</p>
<blockquote class="citation">
<p>
I think that it should not be the case because python ints/long are smaller and faster than Sage Integers, and that we should not be forced to instantiate Integers if we do not need to.
</p>
<p>
For the very same reason you invoked, earlier, when you said that you did not want rationals because of the possible loss of time.
</p>
</blockquote>
<p>
It was not only loss of time but well defineness. Additioning Python int with Sage integers is slower than Sage integer with Sage integer.
</p>
<pre class="wiki">sage: python_one = 1r
sage: sage_one = 1
sage: sage_a = 2**12
sage: timeit("sage_a + python_one", number=5000000)
5000000 loops, best of 3: 66.3 ns per loop
sage: timeit("sage_a + sage_one", number=5000000)
5000000 loops, best of 3: 57.9 ns per loop
</pre><p>
So it is better to have everything being Sage integers.
</p>
<p>
On the other hand, if you really want Python integers you should call <code>len(my_set)</code> (and be sure to obtain a Python integer) and not <code>my_set.cardinality()</code>.
</p>
<p>
Vincent
</p>
TicketvdelecroixSat, 11 Apr 2015 17:38:57 GMT
https://trac.sagemath.org/ticket/18159#comment:16
https://trac.sagemath.org/ticket/18159#comment:16
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18159#comment:13" title="Comment 13">ncohen</a>:
</p>
<blockquote class="citation">
<p>
(note that your problems are solved if you specify ZZ when you build your vector, isn't it?)
</p>
</blockquote>
<p>
Yes, and
</p>
<pre class="wiki">sage: vector(ZZ, ['1', gap(2), pari(3)])
(1, 2, 3)
</pre><p>
works too.
</p>
TicketgitSat, 11 Apr 2015 17:41:24 GMTcommit changed
https://trac.sagemath.org/ticket/18159#comment:17
https://trac.sagemath.org/ticket/18159#comment:17
<ul>
<li><strong>commit</strong>
changed from <em>17924b8da8777805ca17289e00e62c3beca5a146</em> to <em>e40975a50978d76bf1c8ad4a141470dc0b010289</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=e40975a50978d76bf1c8ad4a141470dc0b010289"><span class="icon"></span>e40975a</a></td><td><code>Trac 18159: fix some tests related to TestSuite</code>
</td></tr></table>
TicketvdelecroixSat, 11 Apr 2015 17:44:07 GMTdescription changed
https://trac.sagemath.org/ticket/18159#comment:18
https://trac.sagemath.org/ticket/18159#comment:18
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/18159?action=diff&version=18">diff</a>)
</li>
</ul>
TicketncohenSat, 11 Apr 2015 17:50:23 GMT
https://trac.sagemath.org/ticket/18159#comment:19
https://trac.sagemath.org/ticket/18159#comment:19
<blockquote class="citation">
<p>
So it is better to have everything being Sage integers.
</p>
</blockquote>
<p>
It may be better to have Sage integers if you want, for instance, to add them with other Sage integers. Why would that be sufficient to prevent anyone from returning int?
</p>
<p>
Nathann
</p>
TicketncohenSat, 11 Apr 2015 17:56:00 GMT
https://trac.sagemath.org/ticket/18159#comment:20
https://trac.sagemath.org/ticket/18159#comment:20
<p>
Okay do whatever you like. At some point in this community there was a rule that we should all agree on something to make it pass. Nowadays it's like you just have to convince one of your friends and it can get in.
</p>
<p>
Nathann
</p>
TicketvdelecroixSat, 11 Apr 2015 18:09:08 GMT
https://trac.sagemath.org/ticket/18159#comment:21
https://trac.sagemath.org/ticket/18159#comment:21
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18159#comment:19" title="Comment 19">ncohen</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
So it is better to have everything being Sage integers.
</p>
</blockquote>
<p>
It may be better to have Sage integers if you want, for instance, to add them with other Sage integers. Why would that be sufficient to prevent anyone from returning int?
</p>
</blockquote>
<p>
It is not sufficient. <strong>You</strong> were arguing that Python ints were better because faster. I just showed you that it is not completely right. An argument for <code>X</code> is generally not an argument for <code>Y</code>.
</p>
<p>
The reason to oblige people to return Sage integers or Infinity in the method <code>.cardinality</code> of a <code>Parent</code> is mostly for consistency (things are generally better if specified)
</p>
TicketncohenSat, 11 Apr 2015 18:38:45 GMT
https://trac.sagemath.org/ticket/18159#comment:22
https://trac.sagemath.org/ticket/18159#comment:22
<blockquote class="citation">
<p>
It is not sufficient. <strong>You</strong> were arguing that Python ints were better because faster. I just showed you that it is not completely right. An argument for <code>X</code> is generally not an argument for <code>Y</code>.
</p>
</blockquote>
<p>
All I wanted to point out is that *sometimes* an int is better than an Integer, and that for this reason they should not be forbidden. I do not mean to say that a Python int is always better than a Sage Integer, and quite clearly it is not the case: ints overflows, Integers do not.
</p>
<blockquote class="citation">
<p>
The reason to oblige people to return Sage integers or Infinity in the method <code>.cardinality</code> of a <code>Parent</code> is mostly for consistency.
</p>
</blockquote>
<p>
Couldn't we just use the <code>TestSuite</code> to detect errors? When you see a non-integer then something is most probably wrong and the testsuite should report it. What you are doing right now is simulate a "return type" for a function in a language that does not support it.
</p>
<p>
Nathann
</p>
TicketvdelecroixSat, 11 Apr 2015 19:04:08 GMT
https://trac.sagemath.org/ticket/18159#comment:23
https://trac.sagemath.org/ticket/18159#comment:23
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18159#comment:22" title="Comment 22">ncohen</a>:
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
It is not sufficient. <strong>You</strong> were arguing that Python ints were better because faster. I just showed you that it is not completely right. An argument for <code>X</code> is generally not an argument for <code>Y</code>.
</p>
</blockquote>
<p>
All I wanted to point out is that *sometimes* an int is better than an Integer, and that for this reason they should not be forbidden. I do not mean to say that a Python int is always better than a Sage Integer, and quite clearly it is not the case: ints overflows, Integers do not.
</p>
</blockquote>
<p>
Just to be sure: I do not want to forbid Python integer in general. For the output of <code>.cardinality()</code> I consider that Sage integers are *always* better. Perhaps I am wrong and this can be debated if needed. And, as I already mentioned, you can get Python integer for less effort doing <code>len(my_set)</code>.
</p>
<blockquote class="citation">
<blockquote class="citation">
<p>
The reason to oblige people to return Sage integers or Infinity in the method <code>.cardinality</code> of a <code>Parent</code> is mostly for consistency.
</p>
</blockquote>
<p>
Couldn't we just use the <code>TestSuite</code> to detect errors?
</p>
</blockquote>
<p>
That is my goal. But we do not agree that returning an <code>int</code> for <code>.cardinality()</code> is an error. In many situation, "ensuring" that it is a Sage integer prevent doing wrong things afterwards.
</p>
<blockquote class="citation">
<p>
When you see a non-integer then something is most probably wrong and the testsuite should report it. What you are doing right now is simulate a "return type" for a function in a language that does not support it.
</p>
</blockquote>
<p>
Yes. But having as dependency a ticket "switch from Python to a strongly typed language" would be too much effort. So Python is a fact and we have to deal with it.
</p>
<p>
Let me mention that Python is trying to do some type checking because it is useful:
</p>
<pre class="wiki">sage: class A(object):
....: def __len__(self):
....: return 'haha! well tried!'
sage: a = A()
sage: len(a)
Traceback (most recent call last):
...
TypeError: an integer is required
</pre><p>
(note that <code>a.__len__()</code> returns what you asked it to do).
</p>
<p>
Vincent
</p>
TicketgitSat, 11 Apr 2015 19:33:17 GMTcommit changed
https://trac.sagemath.org/ticket/18159#comment:24
https://trac.sagemath.org/ticket/18159#comment:24
<ul>
<li><strong>commit</strong>
changed from <em>e40975a50978d76bf1c8ad4a141470dc0b010289</em> to <em>a5d1e0e6f8d61d169bc6dcd416b9bb1309e7fe62</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=d2e06089bd674a551bb1bdf399ffaa6373d1ce2f"><span class="icon"></span>d2e0608</a></td><td><code>Trac 18159: fix the three parents</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=a5d1e0e6f8d61d169bc6dcd416b9bb1309e7fe62"><span class="icon"></span>a5d1e0e</a></td><td><code>Trac 18159: some more cleanup in debruijn_sequence.pyx</code>
</td></tr></table>
TicketvdelecroixSat, 11 Apr 2015 19:34:24 GMTstatus changed
https://trac.sagemath.org/ticket/18159#comment:25
https://trac.sagemath.org/ticket/18159#comment:25
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
TicketvdelecroixSat, 11 Apr 2015 19:34:53 GMTdescription changed
https://trac.sagemath.org/ticket/18159#comment:26
https://trac.sagemath.org/ticket/18159#comment:26
<ul>
<li><strong>description</strong>
modified (<a href="/ticket/18159?action=diff&version=26">diff</a>)
</li>
</ul>
TicketjpfloriMon, 18 May 2015 21:10:42 GMT
https://trac.sagemath.org/ticket/18159#comment:27
https://trac.sagemath.org/ticket/18159#comment:27
<p>
I'd say we should ask for others' feeling on sage-devel before making a move here.
Personally I don't mind returning a Sage integer for cardinality, but Nathann does not agree.
So we should try to satisfy a large proportion of Sage users (or let Sage rot forever).
</p>
TicketbrunoTue, 26 May 2015 13:43:03 GMT
https://trac.sagemath.org/ticket/18159#comment:28
https://trac.sagemath.org/ticket/18159#comment:28
<p>
I agree with Vincent's view here: Having <code>.cardinality()</code> return a Sage Integer is better to my mind since then it comes with all the methods for integers. For a <em>basic</em> user, I guess it may be misleading than for some (finite...) sets you can write <code>S.cardinality().factor()</code> while it sometimes results in an error.
</p>
<p>
I understand Nathann's concern on efficiency. Yet, my impression is that the issue is going to arise to <em>advanced</em> or <em>expert</em> users (whatever this means). These users should be capable of switching to <code>len(S)</code> if they feel the need for it. The proposed change actually is an improvement since it provides the user the choice for the return type, while no such choice existed before.
</p>
TicketncohenTue, 26 May 2015 14:48:24 GMT
https://trac.sagemath.org/ticket/18159#comment:29
https://trac.sagemath.org/ticket/18159#comment:29
<blockquote class="citation">
<p>
I understand Nathann's concern on efficiency. Yet, my impression is that the issue is going to arise to <em>advanced</em> or <em>expert</em> users (whatever this means). These users should be capable of switching to <code>len(S)</code> if they feel the need for it. The proposed change actually is an improvement since it provides the user the choice for the return type, while no such choice existed before.
</p>
</blockquote>
<p>
The arguments that I see on this thread are not about whether "cardinality" should return python ints or Sage integers.
</p>
<p>
While the ticket is indeed about <code>.cardinality</code> only, you will see that the arguments given here can be applied to any other Sage function that returns integers.
</p>
<p>
Of course, having *all* such Sage functions return <code>Integer</code> objects is almost impossible to enforce, yet there is an easy way to enforce that for <code>.cardinality</code>, and this is what the branch implements.
</p>
<p>
I do not like what is going on here, but I will not oppose it. You are too many.
</p>
<p>
Nathann
</p>
TicketbrunoTue, 26 May 2015 15:25:43 GMT
https://trac.sagemath.org/ticket/18159#comment:30
https://trac.sagemath.org/ticket/18159#comment:30
<blockquote class="citation">
<p>
The arguments that I see on this thread are not about whether "cardinality" should return python ints or Sage integers.
</p>
<p>
While the ticket is indeed about <code>.cardinality</code> only, you will see that the arguments given here can be applied to any other Sage function that returns integers.
</p>
</blockquote>
<p>
That is not totally exact, since as Vincent mentioned it, you can replace <code>S.cardinality()</code> by <code>len(S)</code> if you want an <code>int</code>, and for efficiency concerns, <code>len(S.set())</code> is the fastest way to get it. Not all functions have this python counterpart.
</p>
<p>
As a user, I very much cares about the consistency in the return types. And I find this <em>very</em> annoying:
</p>
<div class="wiki-code"><div class="code"><pre>sage<span class="p">:</span> S <span class="o">=</span> Set<span class="p">(</span><span class="nb">range</span><span class="p">(</span><span class="mi">2</span><span class="o">^</span><span class="mi">20</span><span class="p">))</span>
sage<span class="p">:</span> T <span class="o">=</span> CartesianProduct<span class="p">(</span>S<span class="p">,</span>S<span class="p">)</span>
sage<span class="p">:</span> T<span class="o">.</span>cardinality<span class="p">()</span><span class="o">.</span>factor<span class="p">()</span>
<span class="mi">2</span><span class="o">^</span><span class="mi">40</span>
sage<span class="p">:</span> S<span class="o">.</span>cardinality<span class="p">()</span><span class="o">.</span>factor<span class="p">()</span>
Traceback <span class="p">(</span>most recent call last<span class="p">):</span>
<span class="o">...</span>
<span class="ne">AttributeError</span><span class="p">:</span> <span class="s">'int'</span> <span class="nb">object</span> has no attribute <span class="s">'factor'</span>
</pre></div></div><p>
Note that I still agree that the same argument may be used at some other places. For instance, let me consider univariate polynomials in sparse representation. I find the following very weird, and would be in favor of changing this behavior:
</p>
<div class="wiki-code"><div class="code"><pre>sage<span class="p">:</span> R<span class="o">.<</span>x<span class="o">></span> <span class="o">=</span> PolynomialRing<span class="p">(</span>ZZ<span class="p">,</span> sparse<span class="o">=</span><span class="bp">True</span><span class="p">)</span>
sage<span class="p">:</span> p <span class="o">=</span> x<span class="o">^</span><span class="p">(</span><span class="mi">2</span><span class="o">^</span><span class="mi">100</span><span class="p">)</span><span class="o">+</span><span class="mi">1</span>
sage<span class="p">:</span> <span class="nb">map</span><span class="p">(</span><span class="k">lambda</span> t<span class="p">:</span> <span class="nb">type</span><span class="p">(</span>t<span class="p">),</span> p<span class="o">.</span>exponents<span class="p">())</span>
<span class="p">[</span><span class="o"><</span><span class="nb">type</span> <span class="s">'int'</span><span class="o">></span><span class="p">,</span> <span class="o"><</span><span class="nb">type</span> <span class="s">'sage.rings.integer.Integer'</span><span class="o">></span><span class="p">]</span>
</pre></div></div><blockquote class="citation">
<p>
I do not like what is going on here, but I will not oppose it. You are too many.
</p>
</blockquote>
<p>
I've understood that (one of) your concern(s) is efficiency issues. But is it the only one? Don't you think that inconsistency also is a problem?
</p>
TicketncohenTue, 26 May 2015 15:28:50 GMT
https://trac.sagemath.org/ticket/18159#comment:31
https://trac.sagemath.org/ticket/18159#comment:31
<p>
Please, do whatever you want. I voiced my opinion one thousand times already.
</p>
<p>
Nathann
</p>
TicketjpfloriWed, 27 May 2015 12:10:31 GMTstatus changed; work_issues set
https://trac.sagemath.org/ticket/18159#comment:32
https://trac.sagemath.org/ticket/18159#comment:32
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
<li><strong>work_issues</strong>
set to <em>rebase?</em>
</li>
</ul>
TicketgitSat, 30 May 2015 17:41:35 GMTcommit changed
https://trac.sagemath.org/ticket/18159#comment:33
https://trac.sagemath.org/ticket/18159#comment:33
<ul>
<li><strong>commit</strong>
changed from <em>a5d1e0e6f8d61d169bc6dcd416b9bb1309e7fe62</em> to <em>b79143d79eeb85ea079b1dd4ea96ce054594b9cc</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=2d83910543eed0620955ca9e76b13cc13f94cf59"><span class="icon"></span>2d83910</a></td><td><code>Trac #18159: add a ._test_cardinality in Sets category</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=001e78128ca195aab58dfb1a81f83a3acf0a8f2f"><span class="icon"></span>001e781</a></td><td><code>Trac #18159: remove a duplicated test</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=aa0fe005d2a672c1113c859a6f9c18a9a1a32786"><span class="icon"></span>aa0fe00</a></td><td><code>Trac #18159: fix cardinality/is_finite in sets/set.py</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=78f11637a9f45d5227d7be1282c74abf6f527669"><span class="icon"></span>78f1163</a></td><td><code>Trac #18159: fix some tests related to TestSuite</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=842a3fe86a7d0ec62cba5502f0d79daa154db5ce"><span class="icon"></span>842a3fe</a></td><td><code>Trac #18159: fix the three parents</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=b79143d79eeb85ea079b1dd4ea96ce054594b9cc"><span class="icon"></span>b79143d</a></td><td><code>Trac #18159: some more cleanup in debruijn_sequence.pyx</code>
</td></tr></table>
TicketvdelecroixSat, 30 May 2015 19:09:53 GMTstatus changed
https://trac.sagemath.org/ticket/18159#comment:34
https://trac.sagemath.org/ticket/18159#comment:34
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
TicketvdelecroixFri, 12 Jun 2015 15:33:50 GMTwork_issues deleted
https://trac.sagemath.org/ticket/18159#comment:35
https://trac.sagemath.org/ticket/18159#comment:35
<ul>
<li><strong>work_issues</strong>
<em>rebase?</em> deleted
</li>
</ul>
TicketjpfloriWed, 01 Jul 2015 12:30:15 GMTstatus changed
https://trac.sagemath.org/ticket/18159#comment:36
https://trac.sagemath.org/ticket/18159#comment:36
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
There are conflicts in <code>src/sage/combinat/crystals/monomial_crystals.py</code>.
</p>
TicketvdelecroixWed, 01 Jul 2015 12:58:56 GMT
https://trac.sagemath.org/ticket/18159#comment:37
https://trac.sagemath.org/ticket/18159#comment:37
<p>
I will not keep it up to date because you like me working. As you can notice, I rebased it 5 weeks ago and nobody cared. Does your message mean that you will review it? There is no need to keep the branch up to date if there is no reviewer... <code>needs_review</code> means needs a reviewer. Not a patchbot log reader.
</p>
<p>
Vincent
</p>
TicketjpfloriWed, 01 Jul 2015 13:06:45 GMT
https://trac.sagemath.org/ticket/18159#comment:38
https://trac.sagemath.org/ticket/18159#comment:38
<p>
I have some time to review it right now.
I'm not making you work for fun... it's just that my dev time is very scarce these days.
And I have no idea of how to fix the second conflict in the file.
</p>
TicketgitWed, 01 Jul 2015 13:15:32 GMTcommit changed
https://trac.sagemath.org/ticket/18159#comment:39
https://trac.sagemath.org/ticket/18159#comment:39
<ul>
<li><strong>commit</strong>
changed from <em>b79143d79eeb85ea079b1dd4ea96ce054594b9cc</em> to <em>0aad2f94b2d59dee992443efe99ef76c847b80c9</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=575b2644bc183f82ffce9b229bd6e6b3a2134059"><span class="icon"></span>575b264</a></td><td><code>Trac #18159: add a ._test_cardinality in Sets category</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=cc87bd5ca5069d6c5d7efb9a61523bf74c2c05dd"><span class="icon"></span>cc87bd5</a></td><td><code>Trac #18159: remove a duplicated test</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=32c403d5abe6944a2b04fc74a1c3c986afed4420"><span class="icon"></span>32c403d</a></td><td><code>Trac #18159: fix cardinality/is_finite in sets/set.py</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=937ebf41b9ba2ec447617e96ccee77513953919d"><span class="icon"></span>937ebf4</a></td><td><code>Trac #18159: fix some tests related to TestSuite</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=eb50b94345e732b879ab5faf5c8016252a17d60c"><span class="icon"></span>eb50b94</a></td><td><code>Trac #18159: fix two parents</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=0aad2f94b2d59dee992443efe99ef76c847b80c9"><span class="icon"></span>0aad2f9</a></td><td><code>Trac #18159: some more cleanup in debruijn_sequence.pyx</code>
</td></tr></table>
TicketvdelecroixWed, 01 Jul 2015 13:16:09 GMTstatus changed
https://trac.sagemath.org/ticket/18159#comment:40
https://trac.sagemath.org/ticket/18159#comment:40
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>needs_review</em>
</li>
</ul>
<p>
Rebased on 6.8.beta6
</p>
TicketjpfloriWed, 01 Jul 2015 13:35:56 GMT
https://trac.sagemath.org/ticket/18159#comment:41
https://trac.sagemath.org/ticket/18159#comment:41
<p>
Is there any reason why the <a class="missing wiki">DeBruijn?</a> squence stuff is included here?
</p>
TicketvdelecroixWed, 01 Jul 2015 13:52:27 GMT
https://trac.sagemath.org/ticket/18159#comment:42
https://trac.sagemath.org/ticket/18159#comment:42
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/18159#comment:41" title="Comment 41">jpflori</a>:
</p>
<blockquote class="citation">
<p>
Is there any reason why the <a class="missing wiki">DeBruijn?</a> squence stuff is included here?
</p>
</blockquote>
<p>
Not really. You can ignore <code>0aad2f9</code> and I will remove it.
</p>
TicketgitWed, 01 Jul 2015 13:54:10 GMTcommit changed
https://trac.sagemath.org/ticket/18159#comment:43
https://trac.sagemath.org/ticket/18159#comment:43
<ul>
<li><strong>commit</strong>
changed from <em>0aad2f94b2d59dee992443efe99ef76c847b80c9</em> to <em>eb50b94345e732b879ab5faf5c8016252a17d60c</em>
</li>
</ul>
<p>
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
</p>
TicketjpfloriWed, 01 Jul 2015 13:56:57 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/18159#comment:44
https://trac.sagemath.org/ticket/18159#comment:44
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Jean-Pierre Flori</em>
</li>
</ul>
<p>
Thanks for removing the unrelated commit.
LGTM.
</p>
TicketvbraunThu, 02 Jul 2015 20:09:24 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/18159#comment:45
https://trac.sagemath.org/ticket/18159#comment:45
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
<li><strong>branch</strong>
changed from <em>u/vdelecroix/18159</em> to <em>eb50b94345e732b879ab5faf5c8016252a17d60c</em>
</li>
</ul>
Ticket