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

Verify consistent conversion of cx for networkx 1.11 and 2.0+ #30

Closed
coleslaw481 opened this issue Jan 14, 2019 · 1 comment
Closed
Assignees
Labels
Milestone

Comments

@coleslaw481
Copy link
Contributor

In NiceCXNetwork.to_networkx() under the module ndex2/nice_cx_network.py verify conversion of networkx object is consistent with documentation and is consistent between two versions of networkx (1.11 & 2.0+)

Current documentation is a bit vague and CartesianCoordinates is incorrect aspect name it should be cartesianLayout:

Returns a NetworkX graph based on the network. Elements in the CartesianCoordinates aspect of the network are transformed to the NetworkX pos attribute. Node name is stored in the networkx node id. Node 'represents' is stored in the networkx node attribute 'represents'
@coleslaw481 coleslaw481 modified the milestone: 3.0.0 Jan 17, 2019
@coleslaw481 coleslaw481 added this to the 3.2.0 milestone Apr 17, 2019
@coleslaw481 coleslaw481 self-assigned this Apr 17, 2019
@coleslaw481
Copy link
Contributor Author

Major refactoring done to NiceCXNetwork.to_networkx() due to numerous issues.

To fix this issue a new parameter mode was added to NiceCXNetwork.to_networkx() with a default value of legacy which kept behavior like before for backwards compatibility.

To get the fixed version set mode to 'default' or None

As part of this fix the following changes were made:

Code in NiceCXNetwork.to_networkx() was pulled out of this method and put into two separate classes:

  • LegacyNetworkXVersionOnePointOneFactory(legacymode=False)

    • Contains logic for conversion when NiceCXNetwork.to_networkx() was invoked with networkx < 2.0 installed.
    • represents is properly added as a node attribute to the resulting graph (legacymode=True disables this)
    • networkx.MultiGraph is now returned so multiple edges are not arbitrarily omitted (legacymode=True disables this)
  • LegacyNetworkXVersionTwoPlusFactory

    • Contains logic for conversion NiceCXNetwork.to_networkx() was invoked with networkx >= 2.0 installed.
    • This method is deprecated, but exists for backwards compatibility
    • This version sets the node ids in the networkx Graph to the name of the node which
      causes many issues.

coleslaw481 added a commit that referenced this issue Apr 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant