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: |
Description
Change History (9)
comment:1 Changed 4 years ago by
- Branch set to u/chapoton/24985
- Commit set to 393f4ef9880e502dab2e2ea66c755490acfd2dc3
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
- Cc embray jdemeyer fbissey added; dimpase removed
comment:3 Changed 4 years ago by
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
Even easier: leave map
and filter
as they are, and add
from past.builtins import filter, map
comment:5 Changed 4 years ago by
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
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: ↓ 8 Changed 4 years ago by
- 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
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
- Branch changed from u/chapoton/24985 to 393f4ef9880e502dab2e2ea66c755490acfd2dc3
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
some py3 fixes in designs