Sage: Ticket #22809: Pass number of variables to polygens
https://trac.sagemath.org/ticket/22809
<p>
The number of generators of a polynomial can be specified as a number:
</p>
<pre class="wiki">sage: PolynomialRing(QQ, 'x', 4).gens()
(x0, x1, x2, x3)
</pre><p>
This ticket should allow doing:
</p>
<pre class="wiki">sage: polygens(QQ, 'x', 4)
(x0, x1, x2, x3)
</pre><p>
Currently it returns <code>TypeError: polygens() takes at most 2 arguments (3 given)</code>.
</p>
en-usSagehttps://trac.sagemath.org/chrome/site/logo_sagemath_trac.png
https://trac.sagemath.org/ticket/22809
Trac 1.1.6mforetsFri, 14 Apr 2017 11:43:22 GMTbranch set
https://trac.sagemath.org/ticket/22809#comment:1
https://trac.sagemath.org/ticket/22809#comment:1
<ul>
<li><strong>branch</strong>
set to <em>u/mforets/pass_number_of_variables_to_polygens</em>
</li>
</ul>
TicketmforetsFri, 14 Apr 2017 11:51:06 GMTcommit set
https://trac.sagemath.org/ticket/22809#comment:2
https://trac.sagemath.org/ticket/22809#comment:2
<ul>
<li><strong>commit</strong>
set to <em>df2e39f6096e63e18a7014717e44a2013450cb36</em>
</li>
</ul>
<p>
apart from that, i'm tempted to propose making this possible too:
</p>
<pre class="wiki">sage: polygens(QQ, 'x', 4, start_at=1)
(x1, x2, x3, x4)
</pre><hr />
<p>
New commits:
</p>
<table class="wiki">
<tr><td><a class="ext-link" href="https://git.sagemath.org/sage.git/commit?id=df2e39f6096e63e18a7014717e44a2013450cb36"><span class="icon"></span>df2e39f</a></td><td><code>add optional argrs to polygens</code>
</td></tr></table>
TicketkcrismanFri, 14 Apr 2017 14:36:10 GMT
https://trac.sagemath.org/ticket/22809#comment:3
https://trac.sagemath.org/ticket/22809#comment:3
<p>
Hmm, I wasn't aware of this other functionality. I have no opinion about it per se, but if you do it, I wonder if you could experiment with what might happen if one did
</p>
<pre class="wiki">L = polygens(QQ, 'x', 4)
for l in L:
SR(l).inject_variable()
</pre><p>
or whatever the right commands would be for that. Or even
</p>
<pre class="wiki">polygens(SR, 'x', 4)
</pre><p>
though I assume that wouldn't work right. See <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/11576" title="enhancement: make it possible to generate sequences of variables easily (needs_work)">#11576</a> for motivation. (Or forget this comment and just be aware of <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/11576" title="enhancement: make it possible to generate sequences of variables easily (needs_work)">#11576</a>, which had a flurry of activity four years ago but hasn't been looked at since.)
</p>
TicketgitFri, 14 Apr 2017 17:01:31 GMTcommit changed
https://trac.sagemath.org/ticket/22809#comment:4
https://trac.sagemath.org/ticket/22809#comment:4
<ul>
<li><strong>commit</strong>
changed from <em>df2e39f6096e63e18a7014717e44a2013450cb36</em> to <em>6d2082c66faeadc4d569afbc02a5b2fb52bc741b</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="https://git.sagemath.org/sage.git/commit/?id=6d2082c66faeadc4d569afbc02a5b2fb52bc741b"><span class="icon"></span>6d2082c</a></td><td><code>allow inject_variables and start_at options</code>
</td></tr></table>
TicketmforetsFri, 14 Apr 2017 17:10:29 GMT
https://trac.sagemath.org/ticket/22809#comment:5
https://trac.sagemath.org/ticket/22809#comment:5
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:3" title="Comment 3">kcrisman</a>:
</p>
<blockquote class="citation">
<p>
Hmm, I wasn't aware of this other functionality. I have no opinion about it per se, but if you do it, I wonder if you could experiment with what might happen if one did
</p>
<pre class="wiki">L = polygens(QQ, 'x', 4)
for l in L:
SR(l).inject_variable()
</pre><p>
or whatever the right commands would be for that. Or even
</p>
<pre class="wiki">polygens(SR, 'x', 4)
</pre><p>
though I assume that wouldn't work right.
</p>
</blockquote>
<p>
great idea! it is very useful that these variables 'exist' in the calling method.
as you can see in revised patch i used <code>get_main_globals</code> to get around the issue.
</p>
<p>
how does it look? does someone suggest other examples?
</p>
<blockquote class="citation">
<p>
See <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/11576" title="enhancement: make it possible to generate sequences of variables easily (needs_work)">#11576</a> for motivation. (Or forget this comment and just be aware of <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/11576" title="enhancement: make it possible to generate sequences of variables easily (needs_work)">#11576</a>, which had a flurry of activity four years ago but hasn't been looked at since.)
</p>
</blockquote>
<p>
ok, thanks for pointing that out! although it seems to me that <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/11576" title="enhancement: make it possible to generate sequences of variables easily (needs_work)">#11576</a> is an advanced version of the simpler (but yet lacking) uses <code>var('x', 4)</code> and <code>var('x', 4, 4)</code> for vectors and matrices respectively. or i'm missing some basic reason why they don't exist already?
</p>
TicketnbruinFri, 14 Apr 2017 18:02:28 GMT
https://trac.sagemath.org/ticket/22809#comment:6
https://trac.sagemath.org/ticket/22809#comment:6
<p>
Don't both return a result and inject bindings (and certainly don't go grabbing the global scope!). It's pretty clear that the behaviour of <code>var</code> to both inject and return variables is a mistake once you see the confusion it causes.
</p>
<p>
Injection of polynomial variables is quite efficiently done with <code>PolynomialRing(R,'x',4).inject_variables()</code>, which prints a message and returns <code>None</code> on purpose.
</p>
<p>
<code>polygen(*args)</code> is just a short form of <code>PolynomialRing(*args).gens()</code> which helps people who know what a polynomial variable is but not what a polynomial ring is. I think it's a bad design decision to muddle this with other features.
</p>
<p>
I'm also against providing options to modify the numbering. There's a good reason why <code>RX=PolynomialRing(R,'x',n)</code> is allowed: it's clearly desirable to make "a polynomial ring in n variables" in various settings. Sage requires names for these variables, but specifying n names is bothersome if you don't care about the names. Hence the indexing. which agrees with what <code>RX.gen(i)</code> does. If you do care what your variable names are called, it's easy to specify a list of names. All kinds of possibilities exist: <code>["x_%s"%s for i in range(n)]</code>, <code>["x^(%s)"%s for i in range(1,n+1)]</code> etc. It's much better to teach people how to get full customizibility than to give them one more option to discover, which will probably not meet their needs if they develop further.
</p>
TicketmforetsFri, 14 Apr 2017 18:53:23 GMT
https://trac.sagemath.org/ticket/22809#comment:7
https://trac.sagemath.org/ticket/22809#comment:7
<p>
@nbruin : thanks for the prompt feedback :) but are you ok with df2e39f or not? notice that <code>*args</code> was missing there on the first place.
</p>
<blockquote class="citation">
<p>
Don't both return a result and inject bindings (and certainly don't go grabbing the global scope!).
</p>
</blockquote>
<p>
is there an alternative to <code>global_scope()</code>? i tried with just <code>R.inject_variables()</code> and it didn't work. i suppose it works via <code>return R.injected_variables()</code>, but then you don't have access to your vector of variables, right? for multiplication and so on, it is useful to have it.
</p>
<blockquote class="citation">
<p>
It's pretty clear that the behaviour of <code>var</code> to both inject and return variables is a mistake once you see the confusion it causes.
</p>
</blockquote>
<p>
could you point me to some threads on this issue, so that i can catch up? for me there are no surprises with <code>var</code>. moreover, i find it practical that the function returns you the variables and/or allows you to use them in the console.
</p>
<blockquote class="citation">
<p>
Injection of polynomial variables is quite efficiently done with <code>PolynomialRing(R,'x',4).inject_variables()</code>, which prints a message and returns <code>None</code> on purpose.
</p>
</blockquote>
<p>
ok, but let's agree it is a rather long construction for people with bad memory (i'm in that group).
</p>
<blockquote class="citation">
<p>
<code>polygen(*args)</code> is just a short form of <code>PolynomialRing(*args).gens()</code> which helps people who know what a polynomial variable is but not what a polynomial ring is. I think it's a bad design decision to muddle this with other features.
</p>
</blockquote>
<p>
yes (essentially i'm on that group, too). that's arguable: why should a feature bother at all? so long as it is consistent with the rest & well documented.. (more on this below)
</p>
<blockquote class="citation">
<p>
If you do care what your variable names are called, it's easy to specify a list of names. All kinds of possibilities exist: <code>["x_%s"%s for i in range(n)]</code>, <code>["x^(%s)"%s for i in range(1,n+1)]</code> etc.
</p>
</blockquote>
<p>
that's definitely not easy.
</p>
<blockquote class="citation">
<p>
It's much better to teach people how to get full customizibility than to give them one more option to discover, which will probably not meet their needs if they develop further.
</p>
</blockquote>
<p>
i get the point about "teaching a man to fish", 100%. at the same time, i'm with the lazy people here: if someone thought the solution and shows you a little example in command help, then i prefer using it rather than working it out myself (saves time, and is less error-prone).
</p>
<p>
for the point on features: what about LaTeX? in general people are happy with lots and lots of customizations ready to be discovered.
</p>
TicketnbruinFri, 14 Apr 2017 19:55:11 GMT
https://trac.sagemath.org/ticket/22809#comment:8
https://trac.sagemath.org/ticket/22809#comment:8
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:7" title="Comment 7">mforets</a>:
</p>
<blockquote class="citation">
<p>
@nbruin : thanks for the prompt feedback :) but are you ok with df2e39f or not? notice that <code>*args</code> was missing there on the first place.
</p>
</blockquote>
<p>
Yes, that looks pretty straightforward. It might be that passing <code>*args</code> on wholesale is problematic (it causes erroneous parameters to be reported with a deeper traceback), but I don't see a big problem immediately. That's also something that is easy to fix later.
</p>
<p>
Concerning your other queries:
</p>
<ul><li>in python it is usual that routines that are called because of their side-effects don't return anything. Compare <code>L.sort()</code> versus <code>sorted(L)</code>
</li><li>just google or search the documentation for the number of examples of <code>y=var('y')</code> etc.
</li><li>I don't think you will find many people who would give LaTeX as an example of a well-designed interface.
</li><li>implementing extra features adds bloat and other maintenance burden, and makes it more confusing for new people, because now they will be confronted with multiple ways of doing things. See "The zen of python". Not all of it applies to math software, but generally the ideas there have worked well to keep python a pleasant and easy to learn language.
</li></ul><p>
That said, nothing prevents you from making your own convenience library functions and maintaining them in your own installation. Once they have been sitting around for a year or two and you're finding that you use them often, you'll have a nice body of code to demonstrate in exactly what way they are more convenient. Practice learns that what seems convenient when you're writing/designing something is often not used much in actual use.
</p>
<p>
Sage is getting large and mature enough that if there is a reasonable way of doing something, we should be hesitant about adding alternative constructions.
</p>
TicketkcrismanFri, 14 Apr 2017 20:10:38 GMT
https://trac.sagemath.org/ticket/22809#comment:9
https://trac.sagemath.org/ticket/22809#comment:9
<blockquote class="citation">
<p>
ok, thanks for pointing that out! although it seems to me that <a class="needs_work ticket" href="https://trac.sagemath.org/ticket/11576" title="enhancement: make it possible to generate sequences of variables easily (needs_work)">#11576</a> is an advanced version of the simpler (but yet lacking) uses var('x', 4) and var('x', 4, 4) for vectors and matrices respectively. or i'm missing some basic reason why they don't exist already?
</p>
</blockquote>
<p>
Because nobody has both implemented and reviewed them. Lots of <a class="ext-link" href="https://en.wikipedia.org/wiki/Law_of_triviality"><span class="icon"></span>bikeshedding</a> around exactly what the defaults and syntax should be.
</p>
TicketmforetsFri, 14 Apr 2017 21:40:06 GMT
https://trac.sagemath.org/ticket/22809#comment:10
https://trac.sagemath.org/ticket/22809#comment:10
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:8" title="Comment 8">nbruin</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:7" title="Comment 7">mforets</a>:
</p>
<blockquote class="citation">
<p>
@nbruin : thanks for the prompt feedback :) but are you ok with df2e39f or not? notice that <code>*args</code> was missing there on the first place.
</p>
</blockquote>
<p>
Yes, that looks pretty straightforward. It might be that passing <code>*args</code> on wholesale is problematic (it causes erroneous parameters to be reported with a deeper traceback), but I don't see a big problem immediately. That's also something that is easy to fix later.
</p>
</blockquote>
<p>
got it. df2e39f does solve the ticket's requirement. yet i suggest to leave it in the current status for 1 week in case someone else want to chime in.
</p>
<p>
BTW I'm willing to learn and apply the more robust solution if you have time now (or in the future).
</p>
<blockquote class="citation">
<p>
That said, nothing prevents you from making your own convenience library functions and maintaining them in your own installation. Once they have been sitting around for a year or two and you're finding that you use them often, you'll have a nice body of code to demonstrate in exactly what way they are more convenient. Practice learns that what seems convenient when you're writing/designing something is often not used much in actual use.
</p>
</blockquote>
<p>
Yes, I need a year or two (more than just time, certainly!) to just grasp what is convenient or not in terms of good design from an expert Python / Sage developer's standpoint.
</p>
<p>
No, I don't need a year or two to know that any of the possibilities that you suggest <code>["x_%s"%s for i in range(n)]</code>, <code>["x^(%s)"%s for i in range(1,n+1)]</code> are not easy. Just because of this:
</p>
<ul><li>Maple (born 1982) -- <code>Vector(n, symbol = x)</code>
</li></ul><ul><li>Matlab (born 1984) -- <code>sym('x', [n 1])</code>
</li></ul><ul><li>Mathematica (born 1988) -- <code>Array[x, n]</code>
</li></ul><ul><li><a class="missing wiki">SymPy?</a> (born 2007) -- <code>symbols('x:n')</code>.
</li></ul><p>
Matrices are constructed in a similar way.
</p>
TicketnbruinFri, 14 Apr 2017 23:29:10 GMT
https://trac.sagemath.org/ticket/22809#comment:11
https://trac.sagemath.org/ticket/22809#comment:11
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:10" title="Comment 10">mforets</a>:
</p>
<blockquote class="citation">
<ul><li>Maple (born 1982) -- <code>Vector(n, symbol = x)</code>
</li></ul><ul><li>Matlab (born 1984) -- <code>sym('x', [n 1])</code>
</li></ul><ul><li>Mathematica (born 1988) -- <code>Array[x, n]</code>
</li></ul><ul><li><a class="missing wiki">SymPy?</a> (born 2007) -- <code>symbols('x:n')</code>.
</li></ul><p>
Matrices are constructed in a similar way.
</p>
</blockquote>
<p>
Doesn't <code>vector(polygen(QQ,'x',n))</code> and <code>matrix(n,m,polygen(QQ,'x',n*m))</code> fit the bill for that? We need a little more symbols, but that's because sage is more explicit about the ring over which one works.
</p>
TicketmforetsSat, 15 Apr 2017 08:08:09 GMT
https://trac.sagemath.org/ticket/22809#comment:12
https://trac.sagemath.org/ticket/22809#comment:12
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:11" title="Comment 11">nbruin</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:10" title="Comment 10">mforets</a>:
</p>
<blockquote class="citation">
<ul><li>Maple (born 1982) -- <code>Vector(n, symbol = x)</code>
</li></ul><ul><li>Matlab (born 1984) -- <code>sym('x', [n 1])</code>
</li></ul><ul><li>Mathematica (born 1988) -- <code>Array[x, n]</code>
</li></ul><ul><li><a class="missing wiki">SymPy?</a> (born 2007) -- <code>symbols('x:n')</code>.
</li></ul><p>
Matrices are constructed in a similar way.
</p>
</blockquote>
<p>
Doesn't <code>vector(polygen(QQ,'x',n))</code> and <code>matrix(n,m,polygen(QQ,'x',n*m))</code> fit the bill for that? We need a little more symbols, but that's because sage is more explicit about the ring over which one works.
</p>
</blockquote>
<p>
Oh yes, that's a nice construction for <code>\mathcal{M}_{n\times m}(\mathbb{K}[x])</code> (minor: it is the plural, <code>polygens</code>).
</p>
<p>
Another thing that the commercial software have in common is that they follow mainstream Linear Algebra convention about variable names. That construction doesn't, especially:
</p>
<pre class="wiki">sage: matrix(2, 4, polygens(QQ, 'x', 8))
[x0 x1 x2 x3]
[x4 x5 x6 x7]
</pre><p>
What do you people think about the following. There is an open ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/13703" title="enhancement: special matrices (closed: fixed)">#13703</a> which targets common matrix constructors. In the same spirit as the polytope library (type <code>polytopes.?</code>), with some work we can have a comprehensive matrix library, <code>matrices.?</code>, that features in particular:
</p>
<pre class="wiki">sage: matrices.symbolic(2)
[a11 a12]
[a21 a22]
</pre><p>
</p>
TicketnbruinSat, 15 Apr 2017 17:02:51 GMT
https://trac.sagemath.org/ticket/22809#comment:13
https://trac.sagemath.org/ticket/22809#comment:13
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:12" title="Comment 12">mforets</a>:
</p>
<blockquote class="citation">
<p>
What do you people think about the following. There is an open ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/13703" title="enhancement: special matrices (closed: fixed)">#13703</a> which targets common matrix constructors. In the same spirit as the polytope library (type <code>polytopes.?</code>), with some work we can have a comprehensive matrix library, <code>matrices.?</code>, that features in particular:
</p>
<pre class="wiki">sage: matrices.symbolic(2)
[a11 a12]
[a21 a22]
</pre></blockquote>
<p>
What would it be used for? If the matrix is of any significant size, there is not much computation you can do with these things. And if the matrix is small you might as well construct it manually. In any case:
</p>
<ul><li>you'd want to be able to specify what the base ring is (or specify you want elements of SR as entries)
</li><li>once you're making these "universal" matrices, you should probably be able to specify the form (symmetric, antisymmetric, upper triangular, etc.)
</li><li>The labelling should be 0-based. You'd want <code>M[1,2]</code> to be <code>a12</code>.
</li></ul>
TicketmforetsSat, 15 Apr 2017 18:46:04 GMT
https://trac.sagemath.org/ticket/22809#comment:14
https://trac.sagemath.org/ticket/22809#comment:14
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:13" title="Comment 13">nbruin</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:12" title="Comment 12">mforets</a>:
</p>
<blockquote class="citation">
<p>
What do you people think about the following. There is an open ticket <a class="closed ticket" href="https://trac.sagemath.org/ticket/13703" title="enhancement: special matrices (closed: fixed)">#13703</a> which targets common matrix constructors. In the same spirit as the polytope library (type <code>polytopes.?</code>), with some work we can have a comprehensive matrix library, <code>matrices.?</code>, that features in particular:
</p>
<pre class="wiki">sage: matrices.symbolic(2)
[a11 a12]
[a21 a22]
</pre></blockquote>
<p>
What would it be used for?
</p>
</blockquote>
<ul><li>Teaching : undergraduate level or before using <a class="wiki" href="https://trac.sagemath.org/wiki/SageMath">SageMath</a>.
</li><li>Mathematical Modeling : to represent an optimization variable in a semidefinite program.
</li></ul><p>
I'd be delighted to go deeper into any of these use cases, or to further exchange our ideas and ideals about less concrete, open-ended issues that innocent <a class="closed ticket" href="https://trac.sagemath.org/ticket/22809" title="enhancement: Pass number of variables to polygens (closed: fixed)">#22809</a> has raised, such as why this very basic construct exists in all the software cited before, while we shouldn't expect a useful answer for the inverse of a big matrix whose coefficients are variables.
</p>
<p>
But, is this relevant at all to <a class="closed ticket" href="https://trac.sagemath.org/ticket/22809" title="enhancement: Pass number of variables to polygens (closed: fixed)">#22809</a>? Maybe in Sage-devel?
</p>
TicketgitSat, 15 Apr 2017 19:41:21 GMTcommit changed
https://trac.sagemath.org/ticket/22809#comment:15
https://trac.sagemath.org/ticket/22809#comment:15
<ul>
<li><strong>commit</strong>
changed from <em>6d2082c66faeadc4d569afbc02a5b2fb52bc741b</em> to <em>7a09c12b7f8b1ff03fdf2093d1f57d41bd622974</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="https://git.sagemath.org/sage.git/commit/?id=7a09c12b7f8b1ff03fdf2093d1f57d41bd622974"><span class="icon"></span>7a09c12</a></td><td><code>Revert "allow inject_variables and start_at options"</code>
</td></tr></table>
TicketmforetsSat, 15 Apr 2017 19:43:17 GMT
https://trac.sagemath.org/ticket/22809#comment:16
https://trac.sagemath.org/ticket/22809#comment:16
<p>
fair enough about naming conventions or handy constructions. as far as i'm concerned, this ticket is ready for peer-review!
</p>
<p>
side note: i've used <code>git revert</code> as suggested <a class="ext-link" href="http://stackoverflow.com/questions/4114095/how-to-revert-git-repository-to-a-previous-commit"><span class="icon"></span>here</a> trying to avoid messing up history (*). please let me know if this is not the correct way in this context.
</p>
<p>
(*) for the record: checked chapter 1 of the Guide, but didn't find this use case.
</p>
TicketmforetsSat, 15 Apr 2017 19:45:04 GMTstatus changed
https://trac.sagemath.org/ticket/22809#comment:17
https://trac.sagemath.org/ticket/22809#comment:17
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>needs_review</em>
</li>
</ul>
<p>
ps: sorry, next time i'll write the updates message in this place so that you don't receive 2 emails..
</p>
TicketnbruinSat, 15 Apr 2017 22:57:09 GMT
https://trac.sagemath.org/ticket/22809#comment:18
https://trac.sagemath.org/ticket/22809#comment:18
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:16" title="Comment 16">mforets</a>:
</p>
<blockquote class="citation">
<p>
side note: i've used <code>git revert</code> as suggested <a class="ext-link" href="http://stackoverflow.com/questions/4114095/how-to-revert-git-repository-to-a-previous-commit"><span class="icon"></span>here</a> trying to avoid messing up history (*). please let me know if this is not the correct way in this context.
</p>
</blockquote>
<p>
You can do that if you care about preserving in the history that a change was made and rolled back. In a case like this I would just go with the branch that does NOT have the changes that were rolled back (rebase, or otherwise rebuild your branch into the state you want directly). Rebasing becomes a real problem if other people have branches based on your branch. I don't think that's an issue here.
</p>
TicketmforetsSun, 16 Apr 2017 10:29:05 GMT
https://trac.sagemath.org/ticket/22809#comment:19
https://trac.sagemath.org/ticket/22809#comment:19
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:18" title="Comment 18">nbruin</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:16" title="Comment 16">mforets</a>:
</p>
<blockquote class="citation">
<p>
side note: i've used <code>git revert</code> as suggested <a class="ext-link" href="http://stackoverflow.com/questions/4114095/how-to-revert-git-repository-to-a-previous-commit"><span class="icon"></span>here</a> trying to avoid messing up history (*). please let me know if this is not the correct way in this context.
</p>
</blockquote>
<p>
You can do that if you care about preserving in the history that a change was made and rolled back. In a case like this I would just go with the branch that does NOT have the changes that were rolled back (rebase, or otherwise rebuild your branch into the state you want directly). Rebasing becomes a real problem if other people have branches based on your branch. I don't think that's an issue here.
</p>
</blockquote>
<p>
ok. may i ask for some extra help here please, like an example? the one in page 19 of the Guide doesn't seem to treat this case. (i did some wrong stuff with git in days84, so i pretend to be more cautious!.)
</p>
TicketnbruinMon, 17 Apr 2017 18:48:57 GMT
https://trac.sagemath.org/ticket/22809#comment:20
https://trac.sagemath.org/ticket/22809#comment:20
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:19" title="Comment 19">mforets</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:18" title="Comment 18">nbruin</a>:
</p>
<blockquote class="citation">
<p>
Replying to <a class="ticket" href="https://trac.sagemath.org/ticket/22809#comment:16" title="Comment 16">mforets</a>:
</p>
<blockquote class="citation">
<p>
side note: i've used <code>git revert</code> as suggested <a class="ext-link" href="http://stackoverflow.com/questions/4114095/how-to-revert-git-repository-to-a-previous-commit"><span class="icon"></span>here</a> trying to avoid messing up history (*). please let me know if this is not the correct way in this context.
</p>
</blockquote>
<p>
You can do that if you care about preserving in the history that a change was made and rolled back. In a case like this I would just go with the branch that does NOT have the changes that were rolled back (rebase, or otherwise rebuild your branch into the state you want directly). Rebasing becomes a real problem if other people have branches based on your branch. I don't think that's an issue here.
</p>
</blockquote>
<p>
ok. may i ask for some extra help here please, like an example? the one in page 19 of the Guide doesn't seem to treat this case. (i did some wrong stuff with git in days84, so i pretend to be more cautious!.)
</p>
</blockquote>
<p>
In this case it would seem to be that a hard reset (as described on the same page) and a forced push to the branch on this ticket would be fine. Anyone who has work based on your branch will be seriously inconvenienced, but I expect there to be no-one in that group here. Note that your actual change is (-2/+7) lines and that with the revert, you'd be archiving the adding and removal of an additional 42 lines.
</p>
<p>
As is described in many places, as soon as you publish a git branch you should be careful rewriting its history, because you could affect people quite badly. However, in this case, with changes so small and localized, any rebase merge would be really simple (and probably handled automatically), so I'd opt for publishing a cleaner history. People shouldn't really be basing work off branches for tickets that are still developing, unless there's a good reason for it.
</p>
TicketgitMon, 17 Apr 2017 21:04:39 GMTcommit changed
https://trac.sagemath.org/ticket/22809#comment:21
https://trac.sagemath.org/ticket/22809#comment:21
<ul>
<li><strong>commit</strong>
changed from <em>7a09c12b7f8b1ff03fdf2093d1f57d41bd622974</em> to <em>5f1a4da2392e198878f33f3bc8fc118178dd6218</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="https://git.sagemath.org/sage.git/commit/?id=5f1a4da2392e198878f33f3bc8fc118178dd6218"><span class="icon"></span>5f1a4da</a></td><td><code>add optional args to polygens</code>
</td></tr></table>
TicketmforetsTue, 18 Apr 2017 16:01:06 GMTauthor set
https://trac.sagemath.org/ticket/22809#comment:22
https://trac.sagemath.org/ticket/22809#comment:22
<ul>
<li><strong>author</strong>
set to <em>Marcelo Forets</em>
</li>
</ul>
TicketmforetsFri, 05 May 2017 12:49:23 GMTcc changed
https://trac.sagemath.org/ticket/22809#comment:23
https://trac.sagemath.org/ticket/22809#comment:23
<ul>
<li><strong>cc</strong>
<em>tscrim</em> <em>vdelecroix</em> added
</li>
</ul>
<p>
@nbruin : thanks for your detailed answer concerning git reset. i've done what you suggested.
</p>
<p>
All: do you have any further inputs with respect to this ticket? thanks.
</p>
TicketrwsFri, 12 May 2017 07:57:29 GMTcomponent changed
https://trac.sagemath.org/ticket/22809#comment:24
https://trac.sagemath.org/ticket/22809#comment:24
<ul>
<li><strong>component</strong>
changed from <em>calculus</em> to <em>commutative algebra</em>
</li>
</ul>
TicketmforetsMon, 22 May 2017 05:39:54 GMT
https://trac.sagemath.org/ticket/22809#comment:25
https://trac.sagemath.org/ticket/22809#comment:25
<p>
friendly reminder for ticket review -- <a class="ext-link" href="https://trac.sagemath.org/ticket/22809#comment:8"><span class="icon"></span>seemingly green light from Nils five weeks ago</a>.
</p>
<p>
thanks
</p>
TicketSimonKingFri, 26 Jul 2019 07:52:57 GMTstatus changed; reviewer set
https://trac.sagemath.org/ticket/22809#comment:26
https://trac.sagemath.org/ticket/22809#comment:26
<ul>
<li><strong>status</strong>
changed from <em>needs_review</em> to <em>positive_review</em>
</li>
<li><strong>reviewer</strong>
set to <em>Simon King</em>
</li>
</ul>
<p>
Although I have never used <code>polygens</code>, I'd say it is an obviously reasonable idea to allow the same optional arguments that are available for the polynomial ring constructor. It doesn't break anything, the patchbot is happy, and it seems to me that 2 years ago people just forgot to set it to positive review. Doing it now. If Nils want to have his name as a reviewer added, please go ahead.
</p>
TicketvbraunSat, 27 Jul 2019 19:51:26 GMTstatus, branch changed; resolution set
https://trac.sagemath.org/ticket/22809#comment:27
https://trac.sagemath.org/ticket/22809#comment:27
<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/mforets/pass_number_of_variables_to_polygens</em> to <em>5f1a4da2392e198878f33f3bc8fc118178dd6218</em>
</li>
</ul>
Ticket