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

workspace=None cannot be overwritten #16

Open
dimpase opened this issue Sep 5, 2021 · 2 comments
Open

workspace=None cannot be overwritten #16

dimpase opened this issue Sep 5, 2021 · 2 comments

Comments

@dimpase
Copy link
Contributor

dimpase commented Sep 5, 2021

in https://trac.sagemath.org/ticket/31404 the GAP workspace management is broken (I guess it never worked).
Here is how I am trying to fix it:

--- a/src/sage/libs/gap/libgap.pyx
+++ b/src/sage/libs/gap/libgap.pyx
@@ -253,7 +253,10 @@ cdef class SageGap(Gap):
     """
 
     def __init__(self):
-        workspace, _ = get_workspace()
+        workspace, isitthere = get_workspace()
+        if not isitthere:
+            workspace = None
         Gap.__init__(self, gap_root=gap_root(), workspace=workspace,
                      autoload=True)
 
@@ -283,6 +287,10 @@ cdef class SageGap(Gap):
             from . import converters
 
             workspace, workspace_is_up_to_date = get_workspace()
+            if self.workspace is None:
+                # Create new workspace
+                self.workspace = os.path.normpath(workspace)
             if self.workspace == os.path.normpath(workspace):
                 # Save a new workspace if necessary
                 if not workspace_is_up_to_date:

it might be necessary to pass Gap.__init__() the value workspace=None, as only then no attempt is made to load
the non-existent workspace, something that leads to a crash in gappy (see here). (This explains the first chunk of the diff).

However, then I cannot overwrite self.workspace later, when I'd like to save a new workspace, so the second chunk leads to the error

AttributeError: attribute 'workspace' of 'gappy.core.Gap' objects is not writable

An obvious way to handle this would be to change gappy code to have one more constructor parameter to tell it whether
workspace is legit, rather than try to use workspace=None to test this.

Can you think of anything better (avoiding modifying gappy) ?

dimpase added a commit to dimpase/gappy that referenced this issue Sep 6, 2021
this works around the impossiblity modify workspace (path) once
it was set to None. So we never do.

This would resolve issue embray#16
@dimpase
Copy link
Contributor Author

dimpase commented Sep 6, 2021

Together with #17, the following solves the problem for Sage

--- a/src/sage/libs/gap/libgap.pyx
+++ b/src/sage/libs/gap/libgap.pyx
@@ -253,8 +253,8 @@ cdef class SageGap(Gap):
     """
 
     def __init__(self):
-        workspace, _ = get_workspace()
-        Gap.__init__(self, gap_root=gap_root(), workspace=workspace,
+        workspace, isitthere = get_workspace()
+        Gap.__init__(self, gap_root=gap_root(), workspace=workspace, workspace_valid=isitthere,
                      autoload=True)
 
     cpdef initialize(self):

@embray
Copy link
Owner

embray commented Jun 2, 2024

Related also to #1

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

No branches or pull requests

2 participants