#26056 closed enhancement (fixed)

py3: care for .iteritems

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.4
Component: python3 Keywords:
Cc: embray, tscrim Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 1e90626 (Commits) Commit: 1e90626d974d9ba60330a95c0072f5471a0fbca0
Dependencies: Stopgaps:

Description (last modified by chapoton)

possibly duplicate with #25948 (which is stalled) ?

Change History (9)

comment:1 Changed 18 months ago by chapoton

  • Branch set to u/chapoton/26056
  • Commit set to 2c8f1e2fdaa3a6bb7bcdd37ebc110069afe0a5fb
  • Status changed from new to needs_review

New commits:

2c8f1e2py3: some care for .iteritems

comment:2 Changed 18 months ago by chapoton

  • Cc embray tscrim added
  • Description modified (diff)

comment:3 Changed 18 months ago by tscrim

  • Branch changed from u/chapoton/26056 to u/tscrim/26056
  • Commit changed from 2c8f1e2fdaa3a6bb7bcdd37ebc110069afe0a5fb to 1bfad281bee0fb2812bab813e0f3786bf67df611
  • Reviewers set to Travis Scrimshaw

For free_module_element.pyx, the e is assumed to be a dict. So I can get a little bit of a speedup too:

sage: from sage.modules.free_module_element import FreeModuleElement_generic_sparse
sage: def S(R,n):
....:     return FreeModule(R, n, sparse=True)
....:
sage: %timeit FreeModuleElement_generic_sparse(S(RR,5), {0:-1, 2:2/3, 3:pi, 4:oo})
The slowest run took 15.53 times longer than the fastest. This could mean that an intermediate result is being cached.
100000 loops, best of 3: 16.4 µs per loop

vs before

sage: %timeit FreeModuleElement_generic_sparse(S(RR,5), {0:-1, 2:2/3, 3:pi, 4:oo})
The slowest run took 14.90 times longer than the fastest. This could mean that an intermediate result is being cached.
100000 loops, best of 3: 16.8 µs per loop

At the very least, it yields cleaner Cython code.

If my changes are good, then positive review.


New commits:

1bfad28Making Cython know a little bit more information.

comment:4 Changed 18 months ago by chapoton

  • Branch changed from u/tscrim/26056 to public/ticket/26056
  • Commit changed from 1bfad281bee0fb2812bab813e0f3786bf67df611 to 13a28097783cb8ce8c9d2dedcba9c6cb8402b1a1
  • Status changed from needs_review to positive_review

ok, thanks. I have made one more commit that just fixes a wrong syntax inside one raise TypeError.

I allow myself to set to positive.


New commits:

13a2809some minor details in raise

comment:5 Changed 18 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:6 Changed 17 months ago by git

  • Commit changed from 13a28097783cb8ce8c9d2dedcba9c6cb8402b1a1 to 1e90626d974d9ba60330a95c0072f5471a0fbca0

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

1e90626Merge branch 'public/ticket/26056' in 8.4.b2

comment:7 Changed 17 months ago by chapoton

  • Status changed from needs_work to needs_review

Travis, I have doubts about the line

+        cdef dict entries_dict, e

as it seems that e has been renamed entries ?

EDIT: it's ok, in fact

Last edited 17 months ago by chapoton (previous) (diff)

comment:8 Changed 17 months ago by chapoton

  • Status changed from needs_review to positive_review

the rebase was trivial, I am setting back to positive

comment:9 Changed 17 months ago by vbraun

  • Branch changed from public/ticket/26056 to 1e90626d974d9ba60330a95c0072f5471a0fbca0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.