Opened 6 months ago

Closed 5 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 6 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 6 months ago by chapoton

  • Description modified (diff)

comment:3 Changed 6 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 6 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 6 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 6 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 6 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 6 months ago by jdemeyer

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

comment:9 Changed 5 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 5 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.