search

cytoscape / RCy3

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

a bug(?) in .nodeNameToSUID (and consequently setNodePropertyBypass) #37

Closed vttrifonov closed 6 years ago

vttrifonov commented 6 years ago

The function .nodeNameToNodeSUID returns the SUIDs in the order in which they come form getTableColumns, instead of preserving the order given in its node.names argument. This creates a problem for setNodePropetyBypass because it seems to expect the node.SUIDs to be ordered the same as the node.names (see the loop in this function): as it is the values and the SUIDs are not guaranteed to be matched.

A possible fix to .nodeNameToNodeSUID is 

    suids<-with(dict, split(SUID, name))
    node.SUIDs <- suids[match(node.names, names(suids))]
    names(node.SUIDs)<-node.names

instead of 

node.SUIDs <- dict[which(dict$name  %in% node.names),'SUID']

The complication is that there could be multiple SUIDs per name. Of course one has to rework functions that use .nodeNameToNodeSUID because it now returns a list.

I noticed that some other conversion functions in RCy3-utils.R use which. These might have to be corrected to fix any other potential order issues.

AlexanderPico commented 6 years ago

Thank you for the bug report. Your suggested fix checks out. I've applied it to all similar functions and ran some tests. I'll be pushing this change now. You can get a version via github while waiting for bioconductor to update (up to 48 hours).