3-manifolds / Spherogram

Spherogram is a Python module for dealing with the kind of planar diagrams that arise in 3-dimensional topology, such as link and Heegaard diagrams. It a component of the larger SnapPy project.
https://snappy.math.uic.edu/spherogram.html
19 stars 8 forks source link

better python3 compatibility for presentations.py #8

Closed fchapoton closed 3 years ago

fchapoton commented 3 years ago

some details

fchapoton commented 3 years ago

ping ? what about this one ? :mask:

NathanDunfield commented 3 years ago

As with #7, I would prefer reverting to the original form of the comparisons on new lines 90, 100, 129, and 306, etc.

fchapoton commented 3 years ago

really ? using len to test if something is empty or not empty is really not a good idea, as it must go through all the elements to compute the length, instead of just checking if there is at least one. Waste of efficiency, really.

And in my opinion, there is not much loss of clarity, as the test over a list, set, tuple, dict is a rather basic feature of python.

NathanDunfield commented 3 years ago

For Python lists and dequeues, len is O(1) since the length is stored as an attribute of the data structure. I even checked with %timeit for dequeues as it's not clearly specified in the docs.

fchapoton commented 3 years ago

ok, here is a partial move putting back some len(self). But maybe you will not like it either.

NathanDunfield commented 3 years ago

But maybe you will not like it either.

You guess correctly.

culler commented 3 years ago

Just a comment about making code changes for the sake of efficiency at the expense of clarity. The truth is that any optimization that can be easily recognized by a script should be unnecessary. The reason is that the optimization can be just as easily recognized by the byte code compiler as by a code checking script. So the optimization should just be implemented by the byte code compiler, leaving the programmer with the flexibility to write her code for optimal readability instead of optimal speed. Now, that does not mean that the byte compiler actually does recognize these particular optimizations. But one day it will.

fchapoton commented 3 years ago

ok, back to the initial state for these comparisons. This should meet your expectations.

NathanDunfield commented 3 years ago

Thanks, merging in 3..2..1..