Ran yapf on modular/abvar code

sagemath / sage

Main repository of SageMath

https://www.sagemath.org

Other

1.45k stars 481 forks source link

Ran yapf on modular/abvar code #27770

Open 99444c9b-4667-4341-abf6-78bb4dfc7950 opened 5 years ago

99444c9b-4667-4341-abf6-78bb4dfc7950 commented 5 years ago

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

Component: modular forms

Keywords: yapf, abvar

Author: Kevin Lui

Branch/Commit: u/klui/yapf_abvar @ abf0185

Issue created by migration from https://trac.sagemath.org/ticket/27770

99444c9b-4667-4341-abf6-78bb4dfc7950 commented 5 years ago

Branch: u/klui/yapf_abvar

99444c9b-4667-4341-abf6-78bb4dfc7950 commented 5 years ago

New commits:

88bd3b3 ran yapf on abvar directory

99444c9b-4667-4341-abf6-78bb4dfc7950 commented 5 years ago

Commit: 88bd3b3

jdemeyer commented 5 years ago

comment:3

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.

99444c9b-4667-4341-abf6-78bb4dfc7950 commented 5 years ago

comment:4

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

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

ef0bb6f manually checked style

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 88bd3b3 to ef0bb6f

99444c9b-4667-4341-abf6-78bb4dfc7950 commented 5 years ago

comment:6

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')
            ]

jdemeyer commented 5 years ago

comment:7

This is of course bikeshedding, but I would write that like

            decomp = [AbelianVariety(f)
                      for f in self.newform_decomposition('a')]

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

84147ba style change

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from ef0bb6f to 84147ba

99444c9b-4667-4341-abf6-78bb4dfc7950 commented 5 years ago

comment:9

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!

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

`713c3b7`	`ran yapf on abvar directory`
`30f79ad`	`manually checked style`
`abf0185`	`style change`

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 84147ba to abf0185

embray commented 5 years ago

comment:11

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).

embray commented 4 years ago

comment:13

Ticket retargeted after milestone closed

mkoeppe commented 4 years ago

comment:14

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.

mkoeppe commented 3 years ago

comment:16

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

mkoeppe commented 3 years ago

comment:17

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