Opened 6 years ago

Closed 6 years ago

#16752 closed enhancement (fixed)

Cleanup in hypergeometric functions

Reported by: tscrim Owned by: tscrim
Priority: minor Milestone: sage-6.3
Component: misc Keywords:
Cc: Merged in:
Authors: Travis Scrimshaw Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: c41fed7 (Commits) Commit: c41fed737f150222efece90cca78498b4aff26c6
Dependencies: Stopgaps:

Description

In #2516, there was some things that could've use some minor tweaks (python 3 compatibility, doc formatting). This ticket makes those tweaks.

Change History (7)

comment:1 Changed 6 years ago by tscrim

  • Branch set to public/cleanup_from_hypergeometric_functions-16752
  • Commit set to 75133bbbdd284fda68236a4f62eb5fdd26865f5b
  • Status changed from new to needs_review

New commits:

8398442Some tweaks fixing things from #2516.
75133bbLast minor tweak.

comment:2 Changed 6 years ago by nbruin

String formatting via "%" is still perfectly valid in python 3, is more concise, and even faster:

sage: fn="a"
sage: %timeit "{%r}" % fn
10000000 loops, best of 3: 142 ns per loop
sage: %timeit "{{{!r}}}".format(fn)
1000000 loops, best of 3: 236 ns per loop

there's no benefit to replacing it with ".format" formatting unless we actually need facilities more easily expressed with it.

comment:3 Changed 6 years ago by tscrim

I thought they were still planning on deprecating % at some point in python 3, but it seems like the winds have changed to keeping it. I'm somewhat surprised format was that much slower too. I'll revert this change.

comment:4 Changed 6 years ago by git

  • Commit changed from 75133bbbdd284fda68236a4f62eb5fdd26865f5b to c41fed737f150222efece90cca78498b4aff26c6

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

c41fed7Reverted change of using format instead of %.

comment:5 Changed 6 years ago by rws

  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

Thanks for cleaning up!

comment:6 Changed 6 years ago by tscrim

Thanks for doing the review and to Nils too.

comment:7 Changed 6 years ago by vbraun

  • Branch changed from public/cleanup_from_hypergeometric_functions-16752 to c41fed737f150222efece90cca78498b4aff26c6
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.