Opened 3 years ago

Closed 3 years ago

#22351 closed defect (fixed)

PyString_AsString is gone in Python3

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

Description (last modified by chapoton)

The functions PyString_AsString and PyString_AsStringAndSize no longer exist in Python 3 and even Cython does not support them.

Where:

git grep -c "^[^#]*PyString_" *.pyx
src/sage/finance/time_series.pyx:2
src/sage/libs/ntl/ntl_ZZ_pE.pyx:2
src/sage/libs/pynac/pynac.pyx:2
src/sage/matrix/matrix_gfpn_dense.pyx:7
src/sage/misc/parser.pyx:2
src/sage/plot/plot3d/index_face_set.pyx:10
src/sage/rings/rational.pyx:1
src/sage/rings/real_mpfi.pyx:4
src/sage/stats/intlist.pyx:2

Change History (20)

comment:1 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:2 Changed 3 years ago by chapoton

  • Description modified (diff)

comment:3 Changed 3 years ago by chapoton

Any idea of what to do here ?

Just replacing PyString_* by PyBytes_* does not work.

comment:4 Changed 3 years ago by chapoton

  • Branch set to u/chapoton/22351
  • Commit set to 1e3562bf1ca5bce1ad6c669b463bbad22abe9e43
  • Status changed from new to needs_review

Here is a tentative.


New commits:

1e3562btrac 22351 first tentative of getting rid of PyString in pyx files

comment:5 Changed 3 years ago by git

  • Commit changed from 1e3562bf1ca5bce1ad6c669b463bbad22abe9e43 to de041a6930d9c8180a9f59af8d63000fced1eff1

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

de041a6trac 22351 one more PyString to PyBytes

comment:6 Changed 3 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer

comment:7 Changed 3 years ago by jdemeyer

  • Branch changed from u/chapoton/22351 to u/jdemeyer/22351

comment:8 Changed 3 years ago by jdemeyer

  • Commit changed from de041a6930d9c8180a9f59af8d63000fced1eff1 to d4c7ea59720149cb69053e739de06a848883a323

I removed the changes to pynac.pyx to avoid a conflict with #21371.


New commits:

d4c7ea5trac 22351 get rid of PyString in pyx files

comment:9 Changed 3 years ago by chapoton

ok, agreed.

Bot is green, do you think this ticket is now ok ?

comment:10 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to needs_work

No, let me clean some stuff up. I will do that right now.

comment:11 Changed 3 years ago by jdemeyer

matrix_gfpn_dense.pyx needs further changes. I will instead add a comment on #21437.

Version 1, edited 3 years ago by jdemeyer (previous) (next) (diff)

comment:12 Changed 3 years ago by git

  • Commit changed from d4c7ea59720149cb69053e739de06a848883a323 to 85c64584b839fb91533f573031c6738c44aa6f37

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

21a1fcetrac 22351 get rid of PyString in pyx files
85c6458Reviewer fixes

comment:13 Changed 3 years ago by jdemeyer

  • Authors set to Frédéric Chapoton, Jeroen Demeyer
  • Status changed from needs_work to needs_review

comment:14 Changed 3 years ago by jdemeyer

I tried to get rid of some calls to PyBytes_... functions. Please review. I have not ran the testsuite, so let's wait for the patchbot.

comment:15 follow-up: Changed 3 years ago by chapoton

Looks good to me, and the bot is green. Shall we set to positive ?

By the way, did you handle the pynac case in #21371 ?

comment:16 in reply to: ↑ 15 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to positive_review

Replying to chapoton:

By the way, did you handle the pynac case in #21371 ?

Yes.

comment:17 Changed 3 years ago by vbraun

Merge conflict...

comment:18 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

comment:19 Changed 3 years ago by chapoton

  • Branch changed from u/jdemeyer/22351 to public/22351
  • Commit changed from 85c64584b839fb91533f573031c6738c44aa6f37 to ed1a775a0da09024cd4d88ad6c1e46d91aab99bb
  • Status changed from needs_work to positive_review

trivial rebase on 7.6.b5, setting back to positive


New commits:

ed1a775Merge branch 'u/jdemeyer/22351' in 7.6.b5

comment:20 Changed 3 years ago by vbraun

  • Branch changed from public/22351 to ed1a775a0da09024cd4d88ad6c1e46d91aab99bb
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.