Opened 10 months ago

Closed 10 months ago

Last modified 10 months ago

#26665 closed defect (fixed)

python3: 'filter' object is not subscriptable in libs/gap/util.pyx

Reported by: fbissey Owned by:
Priority: major Milestone: sage-8.5
Component: python3 Keywords:
Cc: Merged in:
Authors: François Bissey Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 842c6df (Commits) Commit:
Dependencies: Stopgaps:

Description

Because sage-on-gentoo doesn't use GAP_DIR we are exposed to a code path that is not doctested and that fails with python3.

      File "sage/libs/gap/util.pyx", line 199, in sage.libs.gap.util.initialize (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_6/build/cythonized/sage/libs/gap/util.c:4452)
        s = str_to_bytes(gap_root(), FS_ENCODING, "surrogateescape")
      File "sage/libs/gap/util.pyx", line 171, in sage.libs.gap.util.gap_root (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python3_6/build/cythonized/sage/libs/gap/util.c:4232)
        gapdir = filter(lambda dir:dir.strip().startswith('GAP_DIR'), gap_sh)[0]
    TypeError: 'filter' object is not subscriptable

Change History (10)

comment:1 Changed 10 months ago by fbissey

  • Authors set to François Bissey
  • Branch set to u/fbissey/gap_filter
  • Commit set to e273d65435d4cab288b26ff14dd1d3ccc8469b7b

Still have to write a doctest.


New commits:

e273d65Replace filter for compatibility with python3

comment:2 Changed 10 months ago by fbissey

I am not sure how to write that doctest. To test this properly we have to start a new instance of sage with GAP_ROOT_DIR set to wrong value and then call gap_root. I cannot seem to change the value inside of sage in a way that sticks until the gap_root call.

comment:3 Changed 10 months ago by git

  • Commit changed from e273d65435d4cab288b26ff14dd1d3ccc8469b7b to 842c6dfc15bd04c2e40441fb7b2c23608f210b9f

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

842c6dfFigured out a way of doctesting gap_root alternate code

comment:4 Changed 10 months ago by fbissey

  • Status changed from new to needs_review

Figured something out.

comment:5 Changed 10 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:6 Changed 10 months ago by embray

I'm not wild about the fact that this will just crash with a not-so-useful exception if the file is not found, or the line being searched for in that file is not found. But that was the case before this ticket as well so it's fine. I just thought that was worth pointing out.

comment:7 Changed 10 months ago by fbissey

Hopefully once the new gap package is sorted all that mess will disappear.

comment:8 Changed 10 months ago by vbraun

  • Branch changed from u/fbissey/gap_filter to 842c6dfc15bd04c2e40441fb7b2c23608f210b9f
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:9 Changed 10 months ago by gh-timokau

  • Commit 842c6dfc15bd04c2e40441fb7b2c23608f210b9f deleted
sage: os.system("GAP_ROOT_DIR=/not_a_path sage -c \"sage.libs.gap.util.gap_root()\"")
The gap-4.5.5.spkg (or later) seems to be not installed!

This breaks for me because I set GAP_ROOT_DIR in sage-env which is sourced on sage startup. As far as I understand that is how sage-env and GAP_ROOT_DIR are intended to be used, so the doctest should be changed. Also I would expect this test to add significant overhead for little gain, since it has to go through the whole sage startup.

comment:10 Changed 10 months ago by gh-timokau

Fix in #26758.

Note: See TracTickets for help on using tickets.