Replace 'if x != False' with 'if x', etc.

[jhpalmieri]

There are many instances in the Sage library of code like

if A == True:
...
if B == False:
...
if C == None:
...

which (most of the time) should be changed to

if A:
...
if not B:
...
if C is None:
...

There are some exceptions, for example if B can be True, False, or None, then we may want to use if B is False to distinguish between False and None. But most should be changed.

[jhpalmieri]

New commits:

​864a42e

trac 22889: replace 'if x == True' with 'if x', etc.

[jdemeyer]

I'm not entirely convinced with the != cases. My gut feeling is that x != False should be translated as x is not False. If x can only be True or False, why write x != False instead of the simpler x == True?

What do you think?

[jhpalmieri]

As to why write x != False instead of x == True, sometimes there are several possible values for x, for example as in plot/plot.py -- see below. So I can understand it, but in those cases, if x != False is equivalent to if x. I think the only situation where if x != False should be if x is not False is if x might be None or [], for example, and you want to treat those differently from when x is False. I tried to make sure that wasn't the situation in any of the changes I made.

• in ode.pyx: this file is badly documented, but I think the changes here are okay.

• in linear_code.py, if ans != False: as far as I can tell, ans can be either False or a list of positive length, so if ans != False: is the same as if ans. (ans is set to B1.is_isomorphic(B2), where is_isomorphic comes from sage.groups.perm_gps.partn_ref.refinement_binary. That method is not well documented: what are the possible return values? From the code, it should be either False or a list, as I said. Or can the list have length 0 somehow?)

• in hyperplane_arrangement/plot.py, if hyperplane_legend != False: hyperplane_legend can be a bool or either of the strings "long" or "short", so the change is safe, just as in the previous case.

• assert self._regular!=False: I already changed this to assert self._regular is not False

• in plot/plot.py: detect_poles != False. detect_poles can be True, False or "show". So safe to change.

[jdemeyer]

In any case, needs to be rebased.

[jdemeyer]

Replying to jhpalmieri:

• in linear_code.py, if ans != False: as far as I can tell, ans can be either False or a list of positive length, so if ans != False: is the same as if ans. (ans is set to B1.is_isomorphic(B2), where is_isomorphic comes from sage.groups.perm_gps.partn_ref.refinement_binary. That method is not well documented: what are the possible return values? From the code, it should be either False or a list, as I said. Or can the list have length 0 somehow?)

You don't seem very convinced, so I would prefer to use is False here.

[jhpalmieri]

I just checked: the NonlinearBinaryCodeStruct used in linear_code.py will not produce a length zero list. The length of the list is self.degree, and self.degree is the number of columns in the matrix used to define self. If I plug in a matrix with zero columns, I get this lovely thing:

sage: NonlinearBinaryCodeStruct(matrix(GF(2), 2, 0))
python(69054,0x7fff7dcbe000) malloc: *** error for object 0x1afbd0002: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
------------------------------------------------------------------------
0   signals.so                          0x00000001113747f5 print_backtrace + 37
------------------------------------------------------------------------
Unhandled SIGABRT: An abort() occurred.
This probably occurred because a *compiled* module has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Python will now terminate.
------------------------------------------------------------------------

So the list will always have a positive number of entries. I can still make the change to is False if you want; I don't care that much.

[git]

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

​321e47b

Merge branch 'develop' into t/22889/true-false

[jhpalmieri]

Rebased.

[jdemeyer]

Rebased again and changed linear_code.py.

---

New commits:

​96f09ed	Merge tag '8.0.beta12' into t/22889/true-false
​cf2da7b	Use "ans is False" instead of "not ans"

[git]

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

​75d391c

Check "is False" from result of _indefinite_factorization()

[git]

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

​80e5ff2	Check "is False" from result of _indefinite_factorization()
​3612238	Use "ans is not False" instead of "ans" for result of is_isomorphic()

[vklein]

Some other instances can be changed

Targets
    Occurrences of 'if .* == None' in Directory sage/src/sage
Found Occurrences  (1 usage found)
    sage/schemes/riemann_surfaces  (1 usage found)
        riemann_surface.py  (1 usage found)
            1319if (self._differentials == None):

Targets
    Occurrences of 'if .* == True' in Directory sage/src/sage
Found Occurrences  (3 usages found)
    sage/calculus  (1 usage found)
        calculus.py  (1 usage found)
            866if hold == True:
    sage/interfaces  (1 usage found)
        phc.py  (1 usage found)
            85if output_list[i-3].count('success') > 0 or get_failures == True:
    sage/schemes/product_projective  (1 usage found)
        wehlerK3.py  (1 usage found)
            987if check == True:

Targets
    Occurrences of 'if .*==True' in Directory sage/src/sage
Found Occurrences  (2 usages found)
    sage/coding  (1 usage found)
        code_bounds.py  (1 usage found)
            217if field_based==True and (not is_prime_power(q)):
    sage/matroids  (1 usage found)
        matroid.pyx  (1 usage found)
            7718if matroids_plot_helpers.posdict_is_sane(self,pos_dict) ==True:

Targets
    Occurrences of 'if .*==False' in Directory sage/src/sage
Found Occurrences  (1 usage found)
    sage/schemes/affine  (1 usage found)
        affine_morphism.py  (1 usage found)
            678if is_AffineSpace(self.domain())==False:

[jdemeyer]

This ticket is not claiming to fix everything.

[vklein]

Ok. But is it Ok to fix the ones I listed in this ticket? Or should we open another one?

[jdemeyer]

I'm fine with anything which does not make it harder for this ticket to get positive_review.

[vklein]

Then i will open a new ticket and continue the review ( I still have some tests running).

[vklein]

It's all good to me. Tests are ok.