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: adding .items iterator to vectors #25381

Closed
fchapoton opened this issue May 17, 2018 · 12 comments
Closed

py3: adding .items iterator to vectors #25381

fchapoton opened this issue May 17, 2018 · 12 comments

Comments

@fchapoton
Copy link
Contributor

to help build doc in python3

CC: @embray @jdemeyer

Component: python3

Author: Frédéric Chapoton

Branch/Commit: 2fd6154

Reviewer: Erik Bray

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

@fchapoton fchapoton added this to the sage-8.3 milestone May 17, 2018
@fchapoton
Copy link
Contributor Author

New commits:

cbd1888adding .items method to vectors for py3 sake

@fchapoton
Copy link
Contributor Author

Commit: cbd1888

@fchapoton
Copy link
Contributor Author

Branch: u/chapoton/25381

@embray
Copy link
Contributor

embray commented May 17, 2018

comment:2

+1 in principle.

I have a different approach to this in my python3 branch that I haven't made a ticket for yet. What do you think?

diff --git a/src/sage/modules/free_module_element.pyx b/src/sage/modules/free_module_element.pyx
index 29b2de1..814afa7 100644
--- a/src/sage/modules/free_module_element.pyx
+++ b/src/sage/modules/free_module_element.pyx
@@ -821,6 +821,7 @@ def random_vector(ring, degree=None, *args, **kwds):
     entries = [ring.random_element(*args, **kwds) for _ in range(degree)]
     return vector(ring, degree, entries, sparse)

+
 cdef class FreeModuleElement(Vector):   # abstract base class
     """
     An element of a generic free module.
@@ -1545,6 +1546,28 @@ cdef class FreeModuleElement(Vector):   # abstract base class
         from sage.arith.all import lcm
         return lcm(v)

+    def items(self):
+        """
+        Return an iterable over self.
+
+        On Python 2 this returns the same as ``list(self.iteritems())``, while
+        on Python 3 this is an alias for ``self.items()``.  This is for
+        congruence with ``dict.items()``.
+
+        EXAMPLES::
+
+            sage: v = vector([1, 2/3, pi])
+            sage: v.items()  # py2
+            [(0, 1), (1, 2/3), (2, pi)]
+            sage: v.items()  # py3
+            <generator object at ...>
+        """
+
+        IF PY_MAJOR_VERSION < 3:
+            return list(self.iteritems())
+        ELSE:
+            return self.iteritems()
+
     def iteritems(self):
         """
         Return iterator over self.
@@ -4543,7 +4566,7 @@ cdef class FreeModuleElement_generic_sparse(FreeModuleElement):
                 e = entries
                 entries = {}
                 try:
-                    for k, x in e.iteritems():
+                    for k, x in e.items():
                         x = coefficient_ring(x)
                         if x:
                             entries[k] = x
@@ -4734,11 +4757,11 @@ cdef class FreeModuleElement_generic_sparse(FreeModuleElement):

             sage: v = vector([1,2/3,pi], sparse=True)
             sage: v.iteritems()
-            <dictionary-itemiterator object at ...>
+            <dict...itemiterator object at ...>
             sage: list(v.iteritems())
             [(0, 1), (1, 2/3), (2, pi)]
         """
-        return self._entries.iteritems()
+        return iter(self._entries.iteritems())

     def __reduce__(self):
         """

@fchapoton
Copy link
Contributor Author

comment:3

oh, well. Given that "items" currently does not exist, one can just provide it directly with the python3 behaviour. Otherwise, this opens the possibility that someone will use that "items" in a python3-incompatible way, and that we will have to fix this bad use later..

@embray
Copy link
Contributor

embray commented May 18, 2018

comment:4

That is true--the question is to be compatible with Python 2 semantics on Python 2, or just be immediately forward-compatible with Python 3. Since FreeModuleElement is not actually a dict subclass I think we have some freedom to pick the latter.

In that case though what you might do instead is actually rename the iteritems() method to just items() and make iteritems() and deprecated (perhaps just informally?) alias for items().

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 18, 2018

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

2fd6154trac 25381 iteritems as an alias of items for vectors

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 18, 2018

Changed commit from cbd1888 to 2fd6154

@fchapoton
Copy link
Contributor Author

comment:6

Done.

I do not think that one should deprecate formally anything.

@embray
Copy link
Contributor

embray commented May 18, 2018

comment:7

I'm inclined to agree in this case, though I can't quite articulate why.

@embray
Copy link
Contributor

embray commented May 18, 2018

Reviewer: Erik Bray

@vbraun
Copy link
Member

vbraun commented May 19, 2018

Changed branch from u/chapoton/25381 to 2fd6154

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