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: sage-9.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 klui

  • Branch set to u/klui/yapf_abvar

comment:2 Changed 13 months ago by klui

  • Commit set to 88bd3b3a099dd4169d28af7b7b711e40da3fc8ab
  • Status changed from new to needs_review

New commits:

88bd3b3ran yapf on abvar directory

comment:3 Changed 13 months ago by jdemeyer

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 klui

  • Status changed from needs_review to needs_work

Okay, that's fair. I'll manually check it.

comment:5 Changed 13 months ago by git

  • Commit changed from 88bd3b3a099dd4169d28af7b7b711e40da3fc8ab to ef0bb6f699cb1bbcb32243b7ccdec485c67eb6c3

Branch pushed to git repo; I updated commit sha1. New commits:

ef0bb6fmanually checked style

comment:6 Changed 13 months ago by klui

  • 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 follow-up: Changed 13 months ago by jdemeyer

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 git

  • Commit changed from ef0bb6f699cb1bbcb32243b7ccdec485c67eb6c3 to 84147ba8d8646342f0b5ba38bab9991cbf556e57

Branch pushed to git repo; I updated commit sha1. New commits:

84147bastyle change

comment:9 in reply to: ↑ 7 Changed 13 months ago by klui

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 git

  • Commit changed from 84147ba8d8646342f0b5ba38bab9991cbf556e57 to abf0185f6f77e1a91dbe16fc14b0c9ad1c861539

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

713c3b7ran yapf on abvar directory
30f79admanually checked style
abf0185style change

comment:11 Changed 11 months ago by embray

  • Milestone changed from sage-8.8 to sage-8.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 klui

  • Status changed from needs_review to needs_work

comment:13 Changed 5 months ago by embray

  • Milestone changed from sage-8.9 to sage-9.1

Ticket retargeted after milestone closed

comment:14 Changed 6 weeks ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.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.

Note: See TracTickets for help on using tickets.