Sage: Ticket #19941: Rename rings.finite_rings.constructor to finite_field_constructor
https://trac.sagemath.org/ticket/19941
<p>
As the title says. This is associated with the following sage-devel thread:
</p>
<p>
<a class="ext-link" href="https://groups.google.com/d/topic/sage-devel/S1k8HzZUBIs/discussion"><span class="icon"></span>https://groups.google.com/d/topic/sage-devel/S1k8HzZUBIs/discussion</a>
</p>
<p>
This ticket is in conflict with <a class="closed ticket" href="https://trac.sagemath.org/ticket/17569" title="enhancement: Allow creating finite fields without a variable name (closed: fixed)">#17569</a>, but that ticket is in <code>needs_work</code>, and it is apparently a bit hard to merge/rebase two branches when one touches a file that the other renames.
</p>
<p>
Whichever of the two gets a positive review first, I will handle the merge with the other.
</p>
<p>
Nathann
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/19941
Trac 1.1.6ncohenFri, 22 Jan 2016 12:28:29 GMTstatus changed; commit, branch set
https://trac.sagemath.org/ticket/19941#comment:1
https://trac.sagemath.org/ticket/19941#comment:1
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
<li><strong>commit</strong>
set to <em>944735b847af910a17e6e4dafb771ef83382cf54</em>
</li>
<li><strong>branch</strong>
set to <em>public/19941</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=944735b847af910a17e6e4dafb771ef83382cf54"><span class="icon"></span>944735b</a></td><td><code>trac #19941: Rename rings.finite_rings.constructor to finite_field_constructor</code>
</td></tr></table>
TicketgitFri, 22 Jan 2016 12:35:37 GMTcommit changed
https://trac.sagemath.org/ticket/19941#comment:2
https://trac.sagemath.org/ticket/19941#comment:2
<ul>
<li><strong>commit</strong>
changed from <em>944735b847af910a17e6e4dafb771ef83382cf54</em> to <em>7e1852a6e5d477c14a62631128e83618e860d9a6</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=a486ef53160a6de1e1e8e24eb8a0226707d26566"><span class="icon"></span>a486ef5</a></td><td><code>trac #19941: Rename rings.finite_rings.constructor to finite_field_constructor</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=7e1852a6e5d477c14a62631128e83618e860d9a6"><span class="icon"></span>7e1852a</a></td><td><code>trac #19941: new 'constructor.py' file</code>
</td></tr></table>
TicketgitFri, 22 Jan 2016 12:43:55 GMTcommit changed
https://trac.sagemath.org/ticket/19941#comment:3
https://trac.sagemath.org/ticket/19941#comment:3
<ul>
<li><strong>commit</strong>
changed from <em>7e1852a6e5d477c14a62631128e83618e860d9a6</em> to <em>42718e8d42381ba299eae472417402719577795e</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=961c77eb2a5b0b2840b078466dcfa80aa7894189"><span class="icon"></span>961c77e</a></td><td><code>trac #19941: Rename rings.finite_rings.constructor to finite_field_constructor</code>
</td></tr><tr><td><a class="ext-link" href="http://git.sagemath.org/sage.git/commit/?id=42718e8d42381ba299eae472417402719577795e"><span class="icon"></span>42718e8</a></td><td><code>trac #19941: new 'constructor.py' file</code>
</td></tr></table>
TicketchapotonFri, 22 Jan 2016 19:55:46 GMT
https://trac.sagemath.org/ticket/19941#comment:4
https://trac.sagemath.org/ticket/19941#comment:4
<p>
the change to index.rst seems not to be correct..
</p>
TicketroedSat, 23 Jan 2016 00:48:18 GMT
https://trac.sagemath.org/ticket/19941#comment:5
https://trac.sagemath.org/ticket/19941#comment:5
<p>
I don't agree with this change, for two reasons.
</p>
<p>
First, I intend to implement finite rings that are not finite fields, which will be used as quotients of p-adic extension fields. If the change proposed in this ticket is made then we will need to either create another file for the constructors of such rings, or to suffer a situation analogous to that derided in the thread, where complex lazy fields are defined in a file real_lazy.py.
</p>
<p>
Second, I don't see any benefit to making this change. Finite fields are examples of finite rings; what's wrong with the current name?
</p>
TicketncohenSat, 23 Jan 2016 05:26:40 GMT
https://trac.sagemath.org/ticket/19941#comment:6
https://trac.sagemath.org/ticket/19941#comment:6
<blockquote class="citation">
<p>
Second, I don't see any benefit to making this change. Finite fields are examples of finite rings; what's wrong with the current name?
</p>
</blockquote>
<p>
I have less and less patience for people playing dumb.
</p>
<p>
Would a file named 'object.pyx' still get no reaction from you? Finite Rings are objects too. It is much too general and thus becomes misleading: that's what is wrong.
</p>
<p>
Look at the filename: it tells that you can expect the file contains *the* constructor of finite rings (there is not even a plural to <code>constructor</code>). Do you see <code>FiniteRing</code> inside? No. <code>IntegerModRing</code> (also a ring) is not even inside, it sits in its own file, and neither are other finite rings. Inside of that file, what you expect to see is a class named <code>FiniteRing</code>, and absolutely nothing else. Why would you object to having the constructor of finite fields in a file that bears its name? Do you also object to having <code>IntegerModRing</code> in <code>integer_mod_ring.py</code>? If not, by which sorcery?
</p>
<p>
Inside of <code>rings/finite_rings/</code> you have 5 files in <code>finite_field_*</code>. How odd would it be for the file containing their main constructor to have the same prefix?
</p>
<blockquote class="citation">
<p>
First, I intend to implement finite rings that are not finite fields,
</p>
</blockquote>
<p>
And you will be welcome to create a new file for them when you will get to work.
</p>
<p>
Nathann
</p>
TicketdavidloefflerSat, 23 Jan 2016 13:59:36 GMT
https://trac.sagemath.org/ticket/19941#comment:7
https://trac.sagemath.org/ticket/19941#comment:7
<p>
Nathann: You are out of order, and your comments on this ticket and the associated sage_devel thread fall far below the standard of conduct appropriate for Sage contributors.
</p>
<p>
David Roe is one of Sage's most valued developers and he has worked tirelessly on Sage's code for finite and p-adic fields. You, on the other hand, clearly know nothing about the content of this file, and thus you are reduced to making aggressive, mocking comments about its name, which is invisible to most Sage users anyway. You may as well write to the authors of a paper in a top journal telling them that you don't like the title they've given their paper.
</p>
<p>
Don't you have anything better to do with your time?
</p>
TicketncohenSat, 23 Jan 2016 14:01:48 GMT
https://trac.sagemath.org/ticket/19941#comment:8
https://trac.sagemath.org/ticket/19941#comment:8
<p>
You are the one who makes it personal. I am defending that the path/name of this file does not represent its content, and I explain why. You are welcome to answer the technical comments here. For everything else use emails.
</p>
TicketncohenSat, 23 Jan 2016 14:57:37 GMT
https://trac.sagemath.org/ticket/19941#comment:9
https://trac.sagemath.org/ticket/19941#comment:9
<blockquote class="citation">
<p>
the change to index.rst seems not to be correct..
</p>
</blockquote>
<p>
Arggggg... Right!
</p>
<p>
Actually, I just figured out why the file did not seem to be included in the reference manual already: there is an independent 'finite_rings' folder in the manual, which for some reason is not a subfolder of the 'rings' folder.
</p>
<blockquote>
<p>
<a class="ext-link" href="http://doc.sagemath.org/html/en/reference/rings/"><span class="icon"></span>http://doc.sagemath.org/html/en/reference/rings/</a>
<a class="ext-link" href="http://doc.sagemath.org/html/en/reference/finite_rings/"><span class="icon"></span>http://doc.sagemath.org/html/en/reference/finite_rings/</a>
</p>
</blockquote>
<p>
Anyway. That's fixed, and the doc compiles.
</p>
<p>
Nathann
</p>
TicketgitSat, 23 Jan 2016 14:57:54 GMTcommit changed
https://trac.sagemath.org/ticket/19941#comment:10
https://trac.sagemath.org/ticket/19941#comment:10
<ul>
<li><strong>commit</strong>
changed from <em>42718e8d42381ba299eae472417402719577795e</em> to <em>0bc4fc75c99ea6f968e2c72fd71fd33157ed5dd9</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=0bc4fc75c99ea6f968e2c72fd71fd33157ed5dd9"><span class="icon"></span>0bc4fc7</a></td><td><code>trac #19941: Wrong name in index.rst</code>
</td></tr></table>
TicketdimpaseSat, 23 Jan 2016 23:58:36 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/19941#comment:11
https://trac.sagemath.org/ticket/19941#comment:11
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Dima Pasechnik</em>
</li>
</ul>
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/19941#comment:7" title="Comment 7">davidloeffler</a>:
</p>
<blockquote class="citation">
<p>
Nathann: You are out of order, and your comments on this ticket and the associated sage_devel thread fall far below the standard of conduct appropriate for Sage contributors.
</p>
<p>
David Roe is one of Sage's most valued developers and he has worked tirelessly on Sage's code for finite and p-adic fields.
</p>
</blockquote>
<p>
David, FYI, Nathann just as well has been contributing to Sage quite a bit.
</p>
<blockquote class="citation">
<p>
You, on the other hand, clearly know nothing about the content of this file,
</p>
</blockquote>
<p>
David, let me assure you that Nathann certainly perfectly aware of what a finite field is. He has been using them quite a bit...
</p>
<blockquote class="citation">
<p>
and thus you are reduced to making aggressive, mocking comments about its name, which is invisible to most Sage users anyway. You may as well write to the authors of a paper in a top journal telling them that you don't like the title they've given their paper.
</p>
</blockquote>
<p>
There is quite a bit of crap in top journals, as we all know, often due to reviewers...
Thanks goodness, here we can correct things overlooked by Sage reviewers. This is often thankless job, but it makes code much more readable. And this is very important.
</p>
<blockquote class="citation">
<p>
Don't you have anything better to do with your time?
</p>
</blockquote>
<p>
Improving Sage codebase is very important, and we should be thankful to Nathann for this.
</p>
TicketvbraunSun, 24 Jan 2016 06:17:14 GMTstatus changed
https://trac.sagemath.org/ticket/19941#comment:12
https://trac.sagemath.org/ticket/19941#comment:12
<ul>
<li><strong>status</strong>
changed from <em>positive_review</em> to <em>needs_work</em>
</li>
</ul>
<p>
Merge conflict...
</p>
TicketgitSun, 24 Jan 2016 08:53:37 GMTcommit changed
https://trac.sagemath.org/ticket/19941#comment:13
https://trac.sagemath.org/ticket/19941#comment:13
<ul>
<li><strong>commit</strong>
changed from <em>0bc4fc75c99ea6f968e2c72fd71fd33157ed5dd9</em> to <em>bd064a69b3574c77263ef98b999d44e84171e9ea</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=bd064a69b3574c77263ef98b999d44e84171e9ea"><span class="icon"></span>bd064a6</a></td><td><code>trac #19941: Merged with 7.1.beta0</code>
</td></tr></table>
TicketncohenSun, 24 Jan 2016 08:55:40 GMT
https://trac.sagemath.org/ticket/19941#comment:14
https://trac.sagemath.org/ticket/19941#comment:14
<p>
I just fixed the merge problem. Considering what this ticket does, I think it's best to wait for the patchbot to run the tests again, just to be sure <code>^^;</code>
</p>
TicketgitSun, 24 Jan 2016 08:58:47 GMTcommit changed
https://trac.sagemath.org/ticket/19941#comment:15
https://trac.sagemath.org/ticket/19941#comment:15
<ul>
<li><strong>commit</strong>
changed from <em>bd064a69b3574c77263ef98b999d44e84171e9ea</em> to <em>25a961f6e41e97bcebfab2057749b676eb8002c6</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=25a961f6e41e97bcebfab2057749b676eb8002c6"><span class="icon"></span>25a961f</a></td><td><code>trac #19941: Imports from the latest beta</code>
</td></tr></table>
TicketncohenSun, 24 Jan 2016 15:38:17 GMTstatus changed
https://trac.sagemath.org/ticket/19941#comment:16
https://trac.sagemath.org/ticket/19941#comment:16
<ul>
<li><strong>status</strong>
changed from <em>needs_work</em> to <em>positive_review</em>
</li>
</ul>
<p>
It seems to be okay. The broken doctests reported by the patchbot are unrelated.
</p>
<p>
Nathann
</p>
TicketvbraunThu, 28 Jan 2016 17:14:24 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/19941#comment:17
https://trac.sagemath.org/ticket/19941#comment:17
<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>public/19941</em> to <em>25a961f6e41e97bcebfab2057749b676eb8002c6</em>
</li>
</ul>
Ticket