- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 684
some care about algebra_generators #41038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
| Documentation preview for this PR (built with commit 5e9e980; changes) is ready! 🎉 | 
| LGTM. Thank you. Now it fixes the infinite loop bug in python 3.14 | 
| It is the right output? | 
| yes, this is the expected output. | 
| Looking at the behavior, the reason for the change (test failure in Jordan algebra) is that in Python 3.14,  This change is caused by python/cpython#127758 which just modify the internal implementation of a function that converts from a sequence to a tuple. | 
sagemathgh-41038: some care about algebra_generators add type annotations to some of them also change some of them to return Family objects or tuple related to sagemath#41028 for Jordan algebras ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have created tests covering the changes. - [x I have updated the documentation and checked the documentation preview. URL: sagemath#41038 Reported by: Frédéric Chapoton Reviewer(s):
| I am -1 on this change as the output of  | 
| 
 there are infinitely many generators. the current behavior (Python ≤ 3.13) is to raise an error, because  Maybe you'd prefer raising an error (and maybe tell the user to look at  ( @vbraun note that this pull request is un-positive-reviewed for now. I don't think you have automatic notification on status change after first positive review. ) | 
| 
 Yes, I think that proposed behavior is best. (There is also an argument for moving away from the ambiguous  | 
| in the case of  On the other hand, we certainly are including some redundant entries in the list of algebra generators, for example so  | 
| Yes, I agree that there are redundant generators, but this is allowed. It is not finitely generated for the  In other words, this is a spanning set for the Jordan subalgebra of  Moreover, determining an exact generating set for  | 
| Maybe we can directly check  | 
sagemathgh-41038: some care about algebra_generators add type annotations to some of them also change some of them to return Family objects or tuple related to sagemath#41028 for Jordan algebras ### 📝 Checklist - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have created tests covering the changes. - [x I have updated the documentation and checked the documentation preview. URL: sagemath#41038 Reported by: Frédéric Chapoton Reviewer(s):
| Here is the list of things to fix: What would be your suggestion ? | 
| I would argue that a Family F behaves as a tuple, as long as we use "a,b,c=tuple(F)`. | 
| @vbraun note again that this pull request is un-positive-reviewed. (looking at the "vbraun pushed a commit..." notification from GitHub.) @fchapoton I don't think all of them have the issue of potentially infinitely many generators, but if there is a common category for algebras, it looks like there ought to be a default implementation in the category for  | 
dc3ce8c    to
    41f217c      
    Compare
  
    | ok, so here is a brand new proposal, in hope that you will agree. I am only changing one file. | 
| indeed, sorry for the mistake, now fixed | 
|  | 
| shoot.. I will fix that, sorry | 
| Strange. Usually the  | 
add some type annotations
add a check for finite length in the
gensmethod for the sake of python 3.14 change of behaviour intuplerelated to #41028 for Jordan algebras
📝 Checklist