Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#22946 closed enhancement (duplicate)

py3: work on unicode in lazy_import

Reported by: chapoton Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: python3 Keywords: python3, unicode
Cc: jdemeyer, tscrim Merged in:
Authors: Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by chapoton)

solving the first problem met in #22945

Change History (22)

comment:1 Changed 4 years ago by chapoton

  • Branch set to u/chapoton/22946
  • Commit set to 48b236416740678b745a6781719d6d82a046c4b0
  • Status changed from new to needs_review

New commits:

48b2364py3: work on unicode in lazy_import

comment:2 Changed 4 years ago by chapoton

  • Keywords python3 unicode added

comment:3 Changed 4 years ago by git

  • Commit changed from 48b236416740678b745a6781719d6d82a046c4b0 to b17148081ccb41e6a11ce4f61c3e485425313b29

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b171480py3: unicode_literal in lazy_import

comment:4 Changed 4 years ago by chapoton

  • Description modified (diff)

comment:5 Changed 4 years ago by chapoton

  • Cc jdemeyer tscrim added

bot is essentially green

comment:6 Changed 4 years ago by tscrim

Could you explain how importing unicode_literal allows you to use unicode in Python3? I would think you should use unicode_literal.

comment:7 Changed 4 years ago by tscrim

Also, is the spacing in the slices PEP8?

comment:8 Changed 4 years ago by chapoton

hmm, well. I have doubts, I must say.

Question 1: This is a pyx file. I nevertheless wonder whether cython understands "unicode" and "str" correctly in py2/py3 contexts

Question 2: I am sure for the +, not so sure about the :

I am only starting to try and touch the unicode question.. IMHO, this will be a large scale mess, maybe only marginally easier than the "cmp" mountain.

comment:9 Changed 4 years ago by tscrim

Question 1 is something Jeroen might be able to answer, I cannot. For Question 2, I would almost say not putting the spaces around the + as it looks somewhat strange. I more strongly disagree with the space after the : in a slice.

Hopefully the unicode problem won't be so pervasive as we don't use unicode in too many places.

comment:10 Changed 4 years ago by git

  • Commit changed from b17148081ccb41e6a11ce4f61c3e485425313b29 to 05545250fd643ab892680d09667306bdb6472a2d

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

015eebbMerge branch 'u/chapoton/22946' in 8.0.b5
0554525trac 22946 undo pep8 change

comment:11 Changed 4 years ago by chapoton

Jeroen, any comment ?

comment:12 Changed 4 years ago by chapoton

ping ?

comment:13 Changed 4 years ago by tscrim

Or could the import of unicode_literals be a change to unicode like print_function is to print?

comment:14 Changed 4 years ago by tscrim

Okay, so I've looked a little more into unicode_literals, and it is a global flag that makes all strings into unicode (i.e., is like doing u'foo'). I'm slightly worried about potential side effects (well, not on my system since I don't have unicode file paths), so is it strictly necessary to include it?

Then again, this is Cython code, so Python rules need not apply.

comment:15 Changed 4 years ago by chapoton

so, is there still some objection to the current state of the branch ? patchbot is green..

comment:16 Changed 4 years ago by tscrim

I am okay with the branch fundamentally, but I am still a little nervous. So this is a positive review, but I don't want to merge it in such a late beta (maybe the next will be an rc). Instead I would want this merged in 8.1.beta0.

comment:17 Changed 4 years ago by jdemeyer

To answer the questions above:

  1. from __future__ import unicode_literals is like changing all strings in the sources from "foo" to u"foo".
  1. Cython does understand str, bytes, unicode, basestring in all Python versions.

Note that this conflicts with #22755.

comment:18 Changed 4 years ago by chapoton

  • Milestone changed from sage-8.0 to sage-8.1

comment:19 Changed 4 years ago by git

  • Commit changed from 05545250fd643ab892680d09667306bdb6472a2d to 8b16e020e8facab115d60868daad9c2adcd478d9

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

8b16e02Merge branch 'u/chapoton/22946' in 8.0.b12

comment:20 Changed 4 years ago by chapoton

  • Milestone changed from sage-8.1 to sage-duplicate/invalid/wontfix

after merging with 8.0.b12, the branch is essentially empty. So I propose to set this to duplicate.

comment:21 Changed 4 years ago by jdemeyer

  • Authors Frédéric Chapoton deleted
  • Resolution set to duplicate
  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to closed

comment:22 Changed 3 years ago by chapoton

  • Branch u/chapoton/22946 deleted
  • Commit 8b16e020e8facab115d60868daad9c2adcd478d9 deleted
Note: See TracTickets for help on using tickets.