cytoscape / RCy3

New version of RCy3, redesigned and collaboratively maintained by Cytoscape developer community
MIT License
48 stars 20 forks source link

save, exports and imports have a regression bug #141

Closed AlexanderPico closed 3 years ago

AlexanderPico commented 3 years ago

These functions were updated awhile back to use the current working directory in the user's R session for cases where relative export filenames are provided.

See old code here: https://github.com/cytoscape/RCy3/blob/73c87518e0322f599a6c6b0fff4b54ca4a623302/R/Session.R#L96

AlexanderPico commented 3 years ago

Note that this is also true for all import functions, e.g., https://github.com/cytoscape/RCy3/blob/73c87518e0322f599a6c6b0fff4b54ca4a623302/R/Styles.R#L186

In this case getAbsSandboxPath probably needs to be fixed. And for import functions, sandboxGetFileInfo needs to be fixed.

yihangx commented 3 years ago

Fixed. The problem is in the .sandboxOp:

https://github.com/cytoscape/RCy3/blob/6fe1644a03491c591f9a4b9d1596d9cd17a7e92e/R/Sandbox.R#L296

The py4cytoscape use os.path.join(),

https://github.com/cytoscape/py4cytoscape/blob/d46f4007e342011d31e669716a431d3fe6c9fa5e/py4cytoscape/sandbox.py#L462

and I used file.path() in RCy3.

os.path.join function will remove redundant directory name of two files, but file.path cannot achieve this.

So I made wrong choice to use basename of file in the previous version.

Now, I think it is fixed. I tested it in the cloud/sandbox, local/sandbox, local/no-sandbox. Please test and double check it.

AlexanderPico commented 3 years ago

Better, but it still doesn't fully work as expected.

The sandbox system identifies the current working directory when the R package is first initialized (e.g., library(RCy3)), but if the working directory changes, which happens all the time in scripts, then the sandbox path is wrong and still uses the old working directory.

This is a loss of functionality.

When running locally (which is 99% of the use cases), users need to be able to specify a working directory at any time and have export and import follow.

AlexanderPico commented 3 years ago

Let me know if this isn't clear. I imagine this might require a major change to how the sandbox works, which might affect py4cy as well. I this is the case, then let's discuss and present a proposed fix to Barry as well.

yihangx commented 3 years ago

Try this fixed version.

AlexanderPico commented 3 years ago

works!