#19322: a much faster longest_common_prefix for words
-------------------------------------+-------------------------------------
Reporter: vdelecroix | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.9
Component: combinatorics | Resolution:
Keywords: | Merged in:
Authors: Vincent Delecroix | Reviewers:
Report Upstream: N/A | Work issues:
Branch: | Commit:
u/vdelecroix/19322 | fe306f971ac9edde2cec32997c3cff7b4db1ce1b
Dependencies: | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by ncohen):
Hello Vincent,
Looks good. A couple of remarks:
1) You *can* use min/max in Cython code. Contrary to Python `:-P`
2) longest_suffix/Python case: what about 'caching' `len(other)` instead
of recomputing it at every test?
3) longest prefix/Python case: the code generated by python for this
'slice' is scary. Isn't it possible to reimplement it without it? Plus if
you need to import 'islice' it is maybe better to do so at module level,
it's not thaaat bad either and it may happen that the prefix be one
character long, in which case importing stuff could be comparatively more
expensive (?).
4) Have you considered returning a 'new word' (through new_c) even when
that new word is equal to one of self and other? It would simplify the
code, and I don't know if it matters as it does not allocate new memory
anyway?
5) I do not understand your 'not able to initialize a word from [..]'. The
code does not try to initialize a word, does it? It's more something like
"unsupported type"?..
Nathann
--
Ticket URL: <http://trac.sagemath.org/ticket/19322#comment:2>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
