Re: [sage-devel] Re: Governance proposal: Maintainer/code-owner model for .ci, .devcontainer, .github/workflows, tox.ini

[Matthias Koeppe]

On Friday, May 10, 2024 at 4:19:14 PM UTC-7 julian...@fsfe.org wrote:

There are some means to reuse workflows in GitHub


That's correct. 
They are called "reusable workflows", and I use them to provide portability 
testing to upstream projects of Sage. 
You can read about them in https://github.com/sagemath/sage/issues/8
 

(I have not checked if they are feasible for us) and one could certainly 
try to extract some things into shared actions that live elsewhere.


Well, I have checked. No, splitting the repository does not solve the 
problem.

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/4a600f59-5b56-470a-ab40-ad80a5d0a71an%40googlegroups.com.

Re: [sage-devel] Re: Governance proposal: Maintainer/code-owner model for .ci, .devcontainer, .github/workflows, tox.ini

[julian...@fsfe.org]

Dear Matthias,

If I read your proposal correctly, it is about removing review from changes 
made by "maintainers" and merging things directly into develop without 
waiting for the Release Manager.

Mostly, I am opposed to this because changes to the files you list are not 
automatically uncontroversial, see the disputed 
https://github.com/sagemath/sage/pull/37740, 
https://github.com/sagemath/sage/pull/37387, 
https://github.com/sagemath/sage/pull/37351, 
https://github.com/sagemath/sage/pull/36726, 
https://github.com/sagemath/sage/pull/36697, 
https://github.com/sagemath/sage/pull/36694, 
https://github.com/sagemath/sage/pull/36678, (there's probably more.)

In general, I am not sure about changing the review requirements. I know 
that having to do several rounds of review can be frustrating. At the same 
time, I like the idea of having somebody double check things. (I consider 
waiting for the first round of review a necessary annoyance. What can be 
really frustrating is waiting for the second round of review. Maybe, there 
are other ways to improve this, e.g., by encouraging people to set things 
to "feel free to set to positive review yourself once you addressed these 
tiny things I brought up in my review.")

I also think it's fairly confusing to have different rules for different 
parts of the repository. I would not mind at all to use different rules for 
different repositories within the sagemath organization though. And 
breaking the sage repository into smaller bits sounds like a good idea to 
me anyway. (As long as all the src/sage and src/doc stays in one place…) 
There are some means to reuse workflows in GitHub (I have not checked if 
they are feasible for us) and one could certainly try to extract some 
things into shared actions that live elsewhere. Maybe that's an approach 
that is worth exploring?

julian

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/2f6cbb89-d77c-492b-94a6-6b9a901d790cn%40googlegroups.com.

[sage-devel] Re: scalar multiplication of module element fails

['Martin R' via sage-devel]

I think I've got it.  In `free_module_element.pyx` the method 
`FreeModuleElement_generic_dense._lmul_` does the following:

cpdef _lmul_(self, Element right):
"""
EXAMPLES::

sage: v = vector([-1,0,3,pi])   
# needs sage.symbolic
sage: v._lmul_(2/3) 
# needs sage.symbolic
(-2/3, 0, 2, 2/3*pi)
sage: v * (2/3) 
# needs sage.symbolic
(-2/3, 0, 2, 2/3*pi)
"""
if right._parent is self._parent._base:
v = [(x)._mul_(right) for x in self._entries]
else:
v = [x * right for x in self._entries]
return self._new_c(v)

However, symmetric function do not inherit from `RingElement`, which 
implies that the wrong method is called.

What can we do about that?

Martin
On Friday 10 May 2024 at 18:39:21 UTC+2 Martin R wrote:

> Update: I'm afraid I misunderstood  - this does 
> call __mul__, right?
>
> On Friday 10 May 2024 at 15:20:07 UTC+2 Martin R wrote:
>
>> I am trying to fix 
>> https://github.com/sagemath/sage/pull/37976#issuecomment-2104464722
>>
>> Briefly:
>>
>> sage: h = SymmetricFunctions(QQ).h()
>> sage: v = vector([h[3]+h[2,1]])
>> sage: v * (-111)
>> ({[3]: 1, [2, 1]: 1})
>>
>> One reason for this is possibly
>>
>> sage: a = coercion_model.get_action(v.parent(), ZZ, operator.mul, v, -111)
>> sage: a
>> Right scalar multiplication by Integer Ring on Ambient free module of 
>> rank 1 over the integral domain Symmetric Functions over Rational Field in 
>> the homogeneous basis
>> sage: a.op
>> 
>>
>> Help is greatly appreciated, as always,
>>
>> Martin
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/70c0174c-3fbc-4f8b-9809-280d9c316f86n%40googlegroups.com.

[sage-devel] Re: scalar multiplication of module element fails

['Martin R' via sage-devel]

Update: I'm afraid I misunderstood  - this does call 
__mul__, right?

On Friday 10 May 2024 at 15:20:07 UTC+2 Martin R wrote:

> I am trying to fix 
> https://github.com/sagemath/sage/pull/37976#issuecomment-2104464722
>
> Briefly:
>
> sage: h = SymmetricFunctions(QQ).h()
> sage: v = vector([h[3]+h[2,1]])
> sage: v * (-111)
> ({[3]: 1, [2, 1]: 1})
>
> One reason for this is possibly
>
> sage: a = coercion_model.get_action(v.parent(), ZZ, operator.mul, v, -111)
> sage: a
> Right scalar multiplication by Integer Ring on Ambient free module of rank 
> 1 over the integral domain Symmetric Functions over Rational Field in the 
> homogeneous basis
> sage: a.op
> 
>
> Help is greatly appreciated, as always,
>
> Martin
>

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/9a41655c-cc85-4136-836d-bc15d068731dn%40googlegroups.com.

Re: [sage-devel] Re: Governance proposal: Maintainer/code-owner model for .ci, .devcontainer, .github/workflows, tox.ini

[Matthias Koeppe]

On Wednesday, May 8, 2024 at 12:18:51 PM UTC-7 Dima Pasechnik wrote:

I've already said while the previous version of this was discussed, is 
that it's a huge mess to have different commit rights for different 
parts of the tree,


I'm pretty sure that Volker and I can figure this out; there's no need to 
worry about this.

and I proposed to spin the CI into a separate 
repository, as an alternative which simplifies a lot of things.


I responded to this already in 
https://groups.google.com/g/sage-devel/c/dEa3i2Fn3ZY/m/1_43GUDTAAAJ; 
there's nothing to add to that.

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/056adbe1-df6b-4f35-941a-16b06edefc73n%40googlegroups.com.

[sage-devel] Re: New labels v: mimimal, v: small ... on pull requests

[Matthias Koeppe]

FWIW, I suggested to implement this feature 
in https://github.com/sagemath/sage/issues/37254; I'm thankful to Aman Moon 
for implementing this feature and Sebastian Oehms for his help with it. 

Obviously a metric such as the number of lines of changes is only a 
one-dimensional way to express the complexity of a PR. 
When I suggested the feature, I explained the possible positive effects:
- A size label "tiny" could encourage quick reviews of trivial changes.
- A size label "huge" could help flag problematic PRs.
Personally I think that the size labels for "medium-sized" PRs do not add 
much and could be removed.

But I'll note that "developer experience" improvements like this one are 
really best developed exactly as it was done here: By deploying them early, 
the developer community can gain concrete experience with them -- and then 
suggest and implement refinements based on the experience. Harsh dismissals 
of the whole features, on the other hand, are not very helpful.

Matthias

On Thursday, May 9, 2024 at 2:46:17 PM UTC-7 Travis Scrimshaw wrote:

> I am *very* strongly opposed to these tags. Their cutoffs are arbitrary 
> nor they serve no useful purpose as far as I can tell. To this point, they 
> do not reflect the difficulty of a review; in fact, they are at best 
> counterproductive to finding reviewers because it might deter people from 
> reviewing "large" or "huge" changes as they can include lots of trivial 
> doctest changes. At best it is just additional clutter in all of the 
> information for PRs.
>
> From a community perspective, I feel such changes should have been brought 
> to the attention of sage-devel once the PR was at a positive review. 
> Specifically, *before* the PR was merged. Not everyone has time to read 
> every PR, and a small consensus of developers might not reflect the 
> development community at-large when making changes like this.
>
> Best,
> Travis
>
>
> On Tuesday, May 7, 2024 at 3:12:27 PM UTC+9 seb@gmail.com wrote:
>
>> Dear Sage developers,
>>
>> You may have noticed that since yesterday a new type of labels with the 
>> `v:` prefix has appeared on our PRs. These are automatically set to 
>> classify PRs based on their size. For more information, see #37262 
>> .
>>
>> Sebastian
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/67b6b2e6-af69-485e-8509-a2ab86fcd256n%40googlegroups.com.

Re: [sage-devel] Re: New labels v: mimimal, v: small ... on pull requests

[Dima Pasechnik]

On 9 May 2024 22:46:17 BST, Travis Scrimshaw  wrote:
>I am *very* strongly opposed to these tags. Their cutoffs are arbitrary nor 
>they serve no useful purpose as far as I can tell. To this point, they do 
>not reflect the difficulty of a review; in fact, they are at best 
>counterproductive to finding reviewers because it might deter people from 
>reviewing "large" or "huge" changes as they can include lots of trivial 
>doctest changes. At best it is just additional clutter in all of the 
>information for PRs.

It's also discouraging word, "minimal" - you do a "minimal" PR like 

- which potentially implies that we have thousands of missing "volatile" 
declarations in Cython code interfacing libgap, libsingular, and other 
libraries using setjmp/longjmp mechanics to gracefully process errors.


>
>From a community perspective, I feel such changes should have been brought 
>to the attention of sage-devel once the PR was at a positive review. 
>Specifically, *before* the PR was merged. Not everyone has time to read 
>every PR, and a small consensus of developers might not reflect the 
>development community at-large when making changes like this.
>
>Best,
>Travis
>
>
>On Tuesday, May 7, 2024 at 3:12:27 PM UTC+9 seb@gmail.com wrote:
>
>> Dear Sage developers,
>>
>> You may have noticed that since yesterday a new type of labels with the 
>> `v:` prefix has appeared on our PRs. These are automatically set to 
>> classify PRs based on their size. For more information, see #37262 
>> .
>>
>> Sebastian
>>
>

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/A5AA8226-A33F-46DA-AE56-4B66A014930F%40gmail.com.

[sage-devel] scalar multiplication of module element fails

['Martin R' via sage-devel]

I am trying to fix 
https://github.com/sagemath/sage/pull/37976#issuecomment-2104464722

Briefly:

sage: h = SymmetricFunctions(QQ).h()
sage: v = vector([h[3]+h[2,1]])
sage: v * (-111)
({[3]: 1, [2, 1]: 1})

One reason for this is possibly

sage: a = coercion_model.get_action(v.parent(), ZZ, operator.mul, v, -111)
sage: a
Right scalar multiplication by Integer Ring on Ambient free module of rank 
1 over the integral domain Symmetric Functions over Rational Field in the 
homogeneous basis
sage: a.op


Help is greatly appreciated, as always,

Martin

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/27e4abca-5fb7-4723-9d18-cf5df673abfbn%40googlegroups.com.

Re: [sage-devel] Re: New labels v: mimimal, v: small ... on pull requests

[seb....@gmail.com]

I must confess that I have not thought about the aspects of these labels 
that Travis points out, but I fully understand these concerns. If they are 
annoying for many developers, the feature can be easily disabled by 
removing corresponding variables from the repository.


Vincent Delecroix schrieb am Freitag, 10. Mai 2024 um 07:45:13 UTC+2:

> I fully agree with Travis. I do not see the added value of these
> additional tags.
>
> On Thu, 9 May 2024 at 23:46, Travis Scrimshaw  wrote:
> >
> > I am *very* strongly opposed to these tags. Their cutoffs are arbitrary 
> nor they serve no useful purpose as far as I can tell. To this point, they 
> do not reflect the difficulty of a review; in fact, they are at best 
> counterproductive to finding reviewers because it might deter people from 
> reviewing "large" or "huge" changes as they can include lots of trivial 
> doctest changes. At best it is just additional clutter in all of the 
> information for PRs.
> >
> > From a community perspective, I feel such changes should have been 
> brought to the attention of sage-devel once the PR was at a positive 
> review. Specifically, before the PR was merged. Not everyone has time to 
> read every PR, and a small consensus of developers might not reflect the 
> development community at-large when making changes like this.
> >
> > Best,
> > Travis
> >
> >
> > On Tuesday, May 7, 2024 at 3:12:27 PM UTC+9 seb@gmail.com wrote:
> >>
> >> Dear Sage developers,
> >>
> >> You may have noticed that since yesterday a new type of labels with the 
> `v:` prefix has appeared on our PRs. These are automatically set to 
> classify PRs based on their size. For more information, see #37262.
> >>
> >> Sebastian
> >
> > --
> > You received this message because you are subscribed to the Google 
> Groups "sage-devel" group.
> > To unsubscribe from this group and stop receiving emails from it, send 
> an email to sage-devel+...@googlegroups.com.
> > To view this discussion on the web visit 
> https://groups.google.com/d/msgid/sage-devel/a1904934-a31f-4ce0-83c2-76cf2d1a70f1n%40googlegroups.com
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"sage-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to sage-devel+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/sage-devel/d09d6718-ba19-4420-9b94-2fdd722bdc72n%40googlegroups.com.