Opened 3 years ago

Closed 3 years ago

#22271 closed enhancement (fixed)

py3 prepare some cases of zip behaviour

Reported by: chapoton Owned by:
Priority: major Milestone: sage-7.6
Component: python3 Keywords:
Cc: Merged in:
Authors: Frédéric Chapoton Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: bbe4117 (Commits) Commit: bbe41176db974f43f5e28c99bb28fdb817f65332
Dependencies: Stopgaps:

Description

as another step to py3

  • take care of a reversed(zip()) in crystals
  • make sure that factorizations can handle iterators

Change History (10)

comment:1 Changed 3 years ago by chapoton

  • Branch set to u/chapoton/22271
  • Commit set to 3ba3a01e150ebf53e87d8fb80a2ac73e5bab1b17
  • Status changed from new to needs_review

New commits:

3ba3a01py3: handle some cases of zip behaviour

comment:2 Changed 3 years ago by jdemeyer

As a general principle, I don't like code of the form

try:
    do_something()
except E:
    raise E("msg")

Especially if msg is meaningless or plain wrong. Just replace it by

do_something()

(I am talking about the raise TypeError("x must be a list") and also raise TypeError("exponents of factors must be integers"))

Last edited 3 years ago by jdemeyer (previous) (diff)

comment:3 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to needs_work

In this case, you probably don't need the conversion to list at all, since enumerate() can handle iterables. This works without conversion to list:

sage: list(enumerate(xrange(5)))
[(0, 0), (1, 1), (2, 2), (3, 3), (4, 4)]

comment:4 Changed 3 years ago by jdemeyer

Thinking even more about it, why not replace this whole block

        if not isinstance(x, (list, tuple)):
            try:
                x = list(x)
            except TypeError:
                raise TypeError("x must be a list")
        for i, t in enumerate(x):
            if not (isinstance(t, tuple) and len(t) == 2):
                raise TypeError("x must be a list of pairs (p, e) with e an integer")
            try:
                x[i] = (t[0], Integer(t[1]))
            except TypeError:
                raise TypeError("exponents of factors must be integers")

by the one-liner

x = [(p, Integer(e)) for (p,e) in x]

comment:5 Changed 3 years ago by git

  • Commit changed from 3ba3a01e150ebf53e87d8fb80a2ac73e5bab1b17 to bbe41176db974f43f5e28c99bb28fdb817f65332

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

bbe4117trac 22271 code simplified

comment:6 Changed 3 years ago by chapoton

  • Status changed from needs_work to needs_review

good idea, done.

comment:7 Changed 3 years ago by jdemeyer

Let's wait for the patchbot.

comment:8 Changed 3 years ago by chapoton

bot seems to be green

comment:9 Changed 3 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:10 Changed 3 years ago by vbraun

  • Branch changed from u/chapoton/22271 to bbe41176db974f43f5e28c99bb28fdb817f65332
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.