Opened 4 years ago

Closed 4 years ago

#24985 closed enhancement (fixed)

py3: some fixes in designs

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.2
Component: python3 Keywords:
Cc: embray, jdemeyer, fbissey Merged in:
Authors: Frédéric Chapoton Reviewers: Erik Bray
Report Upstream: N/A Work issues:
Branch: 393f4ef (Commits, GitHub, GitLab) Commit: 393f4ef9880e502dab2e2ea66c755490acfd2dc3
Dependencies: Stopgaps:

Status badges

Description


Change History (9)

comment:1 Changed 4 years ago by chapoton

  • Branch set to u/chapoton/24985
  • Commit set to 393f4ef9880e502dab2e2ea66c755490acfd2dc3
  • Status changed from new to needs_review

New commits:

393f4efsome py3 fixes in designs

comment:2 Changed 4 years ago by chapoton

  • Cc embray jdemeyer fbissey added; dimpase removed

comment:3 Changed 4 years ago by dimpase

I would be happy to give it a positive review if you merely wrapped the filter() calls in list().

comment:4 Changed 4 years ago by gh-dimpase

Even easier: leave map and filter as they are, and add

from past.builtins import filter, map

comment:5 Changed 4 years ago by chapoton

About using from past anywhere, it's a strong no-go. We should be moving to python3, not trying to stay with python2.

About your irreducible preference for filter about list comprehension (which is a matter of taste), I hope that you can admit that the code here is equivalent to wrapping twice with list.

Once again I ask you to consider the fact that there remains thousands of issues to be fixed, and that my time and yours should be spared.

comment:6 Changed 4 years ago by dimpase

First of all, what is wrong with import past ? Is it going away soon? No.

Second, this change of syntax is akin to going, say, from left actions to right actions in noncommutative algebra. Now imagine I start editing your texts doing such a change, for the purpose of uniformity...


I agree that this is equivalent in py2. But it is not so in py3. And it is great to have a chance to play with it. And you are basically taking this chance away, without asking.

comment:7 follow-up: Changed 4 years ago by embray

  • Reviewers set to Erik Bray
  • Status changed from needs_review to positive_review

It looks fine to me.

I'm not exactly sure why the addition of __future__.division but if the idea is to just add it in every module I support that.

comment:8 in reply to: ↑ 7 Changed 4 years ago by dimpase

Replying to embray:

It looks fine to me.

I'm not exactly sure why the addition of __future__.division but if the idea is to just add it in every module I support that.

Thanks. As to adding __future__.division, this makes sure that the division is correct for py2 and py3.

comment:9 Changed 4 years ago by vbraun

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