Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

py3: fix gapdir fallback #28456

Closed
antonio-rojas opened this issue Sep 5, 2019 · 11 comments
Closed

py3: fix gapdir fallback #28456

antonio-rojas opened this issue Sep 5, 2019 · 11 comments

Comments

@antonio-rojas
Copy link
Contributor

Fix TypeError: 'filter' object is not subscriptable when GAP_ROOT_DIR doesn't exist.

On sage-the-distro GAP_ROOT_DIR points to an existing dir, so this code path is not being doctested.

CC: @fchapoton @jhpalmieri

Component: python3

Author: Antonio Rojas

Branch/Commit: 350a84e

Reviewer: John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/28456

@antonio-rojas antonio-rojas added this to the sage-8.9 milestone Sep 5, 2019
@antonio-rojas
Copy link
Contributor Author

Branch: u/arojas/py3__fix_gapdir_fallback

@antonio-rojas
Copy link
Contributor Author

Author: Antonio Rojas

@antonio-rojas
Copy link
Contributor Author

comment:2

Fix TypeError: 'filter' object is not subscriptable when GAP_ROOT_DIR doesn't exist.

On sage-the-distro GAP_ROOT_DIR points to an existing dir, so this code path is not being doctested.


New commits:

486111dFix gapdir fallback on python 3

@antonio-rojas
Copy link
Contributor Author

Commit: 486111d

@jhpalmieri
Copy link
Member

comment:3

Why not get rid of the filter command altogether? Use

[x for x in gap_sh if x.strip().startswith('GAP_ROOT')][0]

instead? Or this should be faster since we can stop after the first match:

next(x for x in gap_sh if x.strip().startswith('GAP_ROOT'))

(taken from https://stackoverflow.com/questions/2361426/get-the-first-item-from-an-iterable-that-matches-a-condition)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 5, 2019

Changed commit from 486111d to 350a84e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 5, 2019

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

350a84eUse next

@jhpalmieri
Copy link
Member

comment:5

This looks good to me.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@antonio-rojas

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Sep 9, 2019

Changed branch from u/arojas/py3__fix_gapdir_fallback to 350a84e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants