Opened 13 months ago
Last modified 6 weeks ago
#27770 needs_work enhancement
Ran yapf on modular/abvar code
Reported by:  klui  Owned by:  

Priority:  minor  Milestone:  sage9.2 
Component:  modular forms  Keywords:  yapf, abvar 
Cc:  Merged in:  
Authors:  Kevin Lui  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/klui/yapf_abvar (Commits)  Commit:  abf0185f6f77e1a91dbe16fc14b0c9ad1c861539 
Dependencies:  Stopgaps: 
Description
I'm about to make changes to the modular/abvar code so I figure I ran yapf on it first.
Change History (14)
comment:1 Changed 13 months ago by
 Branch set to u/klui/yapf_abvar
comment:2 Changed 13 months ago by
 Commit set to 88bd3b3a099dd4169d28af7b7b711e40da3fc8ab
 Status changed from new to needs_review
comment:3 Changed 13 months ago by
There are some strange formattings IMHO. For example
if not self.is_subvariety_of_ambient_jacobian( ) or not other.is_subvariety_of_ambient_jacobian():
and
elif isinstance( other, ModularAbelianVariety_abstract) and other.is_subvariety(self):
and
decomp = [ AbelianVariety(f) for f in self.newform_decomposition('a') ]
I'm not convinced that all of these are improvements.
comment:4 Changed 13 months ago by
 Status changed from needs_review to needs_work
Okay, that's fair. I'll manually check it.
comment:5 Changed 13 months ago by
 Commit changed from 88bd3b3a099dd4169d28af7b7b711e40da3fc8ab to ef0bb6f699cb1bbcb32243b7ccdec485c67eb6c3
Branch pushed to git repo; I updated commit sha1. New commits:
ef0bb6f  manually checked style

comment:6 Changed 13 months ago by
 Status changed from needs_work to needs_review
I just manually checked it. I mostly changed how yapf handles long sequences of method application. I didn't like how it often ended a line with (
.
This one seems okay?
decomp = [ AbelianVariety(f) for f in self.newform_decomposition('a') ]
comment:7 followup: ↓ 9 Changed 13 months ago by
This is of course bikeshedding, but I would write that like
decomp = [AbelianVariety(f) for f in self.newform_decomposition('a')]
comment:8 Changed 13 months ago by
 Commit changed from ef0bb6f699cb1bbcb32243b7ccdec485c67eb6c3 to 84147ba8d8646342f0b5ba38bab9991cbf556e57
Branch pushed to git repo; I updated commit sha1. New commits:
84147ba  style change

comment:9 in reply to: ↑ 7 Changed 13 months ago by
Replying to jdemeyer:
This is of course bikeshedding, but I would write that like
decomp = [AbelianVariety(f) for f in self.newform_decomposition('a')]
I like that too. Thanks!
comment:10 Changed 11 months ago by
 Commit changed from 84147ba8d8646342f0b5ba38bab9991cbf556e57 to abf0185f6f77e1a91dbe16fc14b0c9ad1c861539
comment:11 Changed 11 months ago by
 Milestone changed from sage8.8 to sage8.9
Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).
comment:12 Changed 9 months ago by
 Status changed from needs_review to needs_work
comment:13 Changed 5 months ago by
 Milestone changed from sage8.9 to sage9.1
Ticket retargeted after milestone closed
comment:14 Changed 6 weeks ago by
 Milestone changed from sage9.1 to sage9.2
Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.
New commits:
ran yapf on abvar directory