Opened 10 months ago
Closed 9 months ago
#26989 closed enhancement (fixed)
some cleanup around if statements
Reported by:  chapoton  Owned by:  

Priority:  minor  Milestone:  sage8.7 
Component:  refactoring  Keywords:  
Cc:  Merged in:  
Authors:  Frédéric Chapoton  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  14b42ef (Commits)  Commit:  14b42ef8eec9140e700c9e31a7de691f58a1c71e 
Dependencies:  Stopgaps: 
Description (last modified by )
There are a few places with code like
if(something):
This is turned into more pythonic shape.
Change History (10)
comment:1 Changed 10 months ago by
 Branch set to u/chapoton/26989
 Commit set to 361932f1a5a4468fd8ae9d63671e694ba0916457
 Status changed from new to needs_review
comment:2 Changed 10 months ago by
 Description modified (diff)
comment:3 Changed 10 months ago by
 Commit changed from 361932f1a5a4468fd8ae9d63671e694ba0916457 to 85a22d37815014ea03121ca3211c6928a65f7df2
Branch pushed to git repo; I updated commit sha1. New commits:
85a22d3  trac 26989 fix doctests

comment:4 Changed 10 months ago by
 Commit changed from 85a22d37815014ea03121ca3211c6928a65f7df2 to 14b42ef8eec9140e700c9e31a7de691f58a1c71e
Branch pushed to git repo; I updated commit sha1. New commits:
14b42ef  trac 26989 fixing the pyflakes warnings

comment:5 Changed 10 months ago by
Why didn't you remove parentheses here?
if (p_up[0] <= p_down[0] or p_down[1] <= p_up[1]):
comment:6 Changed 10 months ago by
Cython style comment: when comparing C pointers, I prefer is
to ==
to make it clear that we are dealing with object identity (pointers are equal). So I would write if r != currRing:
as if r is not currRing:
comment:7 Changed 10 months ago by
Cython performance comment: product()
is not optimized by Cython, so
for (x,y) in product(range(n),range(m)):
is a lot slower than a nested loop
cdef Py_ssize_t x, y for x in range(n): for y in range(m):
I'm not saying that this is necessarily a big problem here, but I thought I should mention it for future reference.
comment:8 Changed 10 months ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:9 Changed 9 months ago by
 Milestone changed from sage8.6 to sage8.7
Retarging tickets optimistically to the next milestone. If you are responsible for this ticket (either its reporter or owner) and don't believe you are likely to complete this ticket before the next release (8.7) please retarget this ticket's milestone to sagepending or sagewishlist.
comment:10 Changed 9 months ago by
 Branch changed from u/chapoton/26989 to 14b42ef8eec9140e700c9e31a7de691f58a1c71e
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
cleanup around some if statements