Opened 4 years ago

Closed 4 years ago

#27268 closed defect (fixed)

py3: fix FriCAS interface

Reported by: mantepse Owned by:
Priority: major Milestone: sage-8.7
Component: python3 Keywords: FriCAS
Cc: chapoton Merged in:
Authors: Martin Rubey Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 9a6ec3d (Commits, GitHub, GitLab) Commit: 9a6ec3d8d28e6e1a345255652ad6fa935517cbda
Dependencies: #25899 Stopgaps:

Status badges

Description (last modified by mantepse)

In python3, dict.items cannot be indexed. The processing of rootOf expression assumed this, so the design pattern must be changed.

Change History (12)

comment:1 Changed 4 years ago by mantepse

Branch: → u/mantepse/py3__fix_fricas_interface

comment:2 Changed 4 years ago by mantepse

Authors: → Martin Rubey
Cc: chapoton added
Commit: → ec661060ac599f6b1c839f7bcf97efa4f772895a
Component: PLEASE CHANGE → python3
Dependencies: → #25899
Description: modified (diff)
Status: new → needs_review
Type: PLEASE CHANGE → defect

New commits:

​ec66106fix treatment of rootOf expressions for python3

comment:3 Changed 4 years ago by slelievre

Might this help with #17908 and #25905? Should there also be a register_symbol for abs?

comment:4 Changed 4 years ago by mantepse

  • #25905 has a bad description - the bug is further down the comments, and is a true fricas bug
  • I didn't know about #17908, thank you for notifying me! Yes, the cure is very likely now simply adding the translation of abs, I'll keep this in mind.
  • this ticket is unrelated to the two above, it only provides a python3 fix. However, to avoid merge conflicts, I'd like to get in this ticket and #26068 first, I'll then do #17908.

comment:5 in reply to:  4 Changed 4 years ago by slelievre

Replying to mantepse:

... thank you for notifying me! ...

You can query Trac for all tickets whose summary contains "fricas":

comment:6 Changed 4 years ago by mantepse

Keywords: FriCAS added

comment:7 Changed 4 years ago by git

Commit: ec661060ac599f6b1c839f7bcf97efa4f772895a → 9a6ec3d8d28e6e1a345255652ad6fa935517cbda

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

​9a6ec3dMerge branch 'develop' of git://trac.sagemath.org/sage into t/27268/py3__fix_fricas_interface

comment:8 Changed 4 years ago by tscrim

Reviewers: → Travis Scrimshaw
Status: needs_review → positive_review

Actually, the problem is this line: (var, poly) = rootOf.items()[i]. The result of items() is a view, which does not support indexing (in a way, think of it like an iterator). However, since your changes work and the code looks cleaner, positive review.

comment:9 in reply to:  8 Changed 4 years ago by mantepse

Replying to tscrim:

Actually, the problem is this line: (var, poly) = rootOf.items()[i].

Yes, the error message and trace did show that.

The result of items() is a view, which does not support indexing (in a way, think of it like an iterator). However, since your changes work and the code looks cleaner, positive review.

General question: should I modify the ticket description once I know the root of the problem? The downside is that the history how the bugfix developed is sometimes not as easy to understand anymore.

comment:10 Changed 4 years ago by tscrim

Yes, in this case it probably is better to modify it so that people have an easier time seeing the issue (rather than reading the comments).

comment:11 Changed 4 years ago by mantepse

Description: modified (diff)

comment:12 Changed 4 years ago by vbraun

Branch: u/mantepse/py3__fix_fricas_interface → 9a6ec3d8d28e6e1a345255652ad6fa935517cbda
Resolution: → fixed
Status: positive_review → closed
Note: See TracTickets for help on using tickets.