Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

py3: care for .iteritems #26056

Closed
fchapoton opened this issue Aug 13, 2018 · 17 comments
Closed

py3: care for .iteritems #26056

fchapoton opened this issue Aug 13, 2018 · 17 comments

Comments

@fchapoton
Copy link
Contributor

possibly duplicate with #25948 (which is stalled) ?

CC: @embray @tscrim

Component: python3

Author: Frédéric Chapoton

Branch/Commit: 1e90626

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/26056

@fchapoton fchapoton added this to the sage-8.4 milestone Aug 13, 2018
@fchapoton
Copy link
Contributor Author

Commit: 2c8f1e2

@fchapoton
Copy link
Contributor Author

New commits:

2c8f1e2py3: some care for .iteritems

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/26056

@fchapoton

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Aug 14, 2018

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 14, 2018

Changed commit from 2c8f1e2 to 1bfad28

@tscrim
Copy link
Collaborator

tscrim commented Aug 14, 2018

Changed branch from u/chapoton/26056 to u/tscrim/26056

@tscrim
Copy link
Collaborator

tscrim commented Aug 14, 2018

comment:3

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.

@fchapoton
Copy link
Contributor Author

comment:4

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

@fchapoton
Copy link
Contributor Author

Changed commit from 1bfad28 to 13a2809

@fchapoton
Copy link
Contributor Author

Changed branch from u/tscrim/26056 to public/ticket/26056

@vbraun
Copy link
Member

vbraun commented Aug 17, 2018

comment:5

Merge conflict

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 27, 2018

Changed commit from 13a2809 to 1e90626

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 27, 2018

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

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

@fchapoton
Copy link
Contributor Author

comment:7

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

@fchapoton
Copy link
Contributor Author

comment:8

the rebase was trivial, I am setting back to positive

@vbraun
Copy link
Member

vbraun commented Aug 30, 2018

Changed branch from public/ticket/26056 to 1e90626

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants