Opened 3 years ago

Last modified 3 months ago

#27770 needs_work enhancement

Ran yapf on modular/abvar code

Reported by: klui Owned by:
Priority: minor Milestone: sage-9.7
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, GitHub, GitLab) Commit: abf0185f6f77e1a91dbe16fc14b0c9ad1c861539
Dependencies: Stopgaps:

Status badges

Description

I'm about to make changes to the modular/abvar code so I figure I ran yapf on it first.

Change History (19)

comment:1 Changed 3 years ago by klui

  • Branch set to u/klui/yapf_abvar

comment:2 Changed 3 years ago by klui

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

New commits:

88bd3b3ran yapf on abvar directory

comment:3 Changed 3 years 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 3 years ago by klui

  • Status changed from needs_review to needs_work

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

comment:5 Changed 3 years 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 3 years 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 3 years 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 3 years 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 3 years 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 3 years 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 3 years 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 3 years ago by klui

  • Status changed from needs_review to needs_work

comment:13 Changed 3 years ago by embray

  • Milestone changed from sage-8.9 to sage-9.1

Ticket retargeted after milestone closed

comment:14 Changed 2 years 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.

comment:15 Changed 22 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:16 Changed 17 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:17 Changed 12 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

Setting a new milestone for this ticket based on a cursory review.

comment:18 Changed 7 months ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.6

comment:19 Changed 3 months ago by mkoeppe

  • Milestone changed from sage-9.6 to sage-9.7
Note: See TracTickets for help on using tickets.