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: sage-8.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 chapoton)

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 chapoton

  • Branch set to u/chapoton/26989
  • Commit set to 361932f1a5a4468fd8ae9d63671e694ba0916457
  • Status changed from new to needs_review

New commits:

361932fcleanup around some if statements

comment:2 Changed 10 months ago by chapoton

  • Description modified (diff)

comment:3 Changed 10 months ago by git

  • Commit changed from 361932f1a5a4468fd8ae9d63671e694ba0916457 to 85a22d37815014ea03121ca3211c6928a65f7df2

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

85a22d3trac 26989 fix doctests

comment:4 Changed 10 months ago by git

  • Commit changed from 85a22d37815014ea03121ca3211c6928a65f7df2 to 14b42ef8eec9140e700c9e31a7de691f58a1c71e

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

14b42eftrac 26989 fixing the pyflakes warnings

comment:5 Changed 10 months ago by jdemeyer

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 jdemeyer

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 jdemeyer

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 jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:9 Changed 9 months ago by embray

  • Milestone changed from sage-8.6 to sage-8.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 sage-pending or sage-wishlist.

comment:10 Changed 9 months ago by vbraun

  • Branch changed from u/chapoton/26989 to 14b42ef8eec9140e700c9e31a7de691f58a1c71e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.