microsoft / vivainsights-py

Python package for Analyzing and Visualizing data from Viva Insights
https://microsoft.github.io/vivainsights-py/
Other
14 stars 2 forks source link

fix: update for network_p2p. #17

Open sachinstl opened 8 months ago

sachinstl commented 8 months ago

Changes in plot, community and centrality using Networkx package.

  1. In new version Plot is based on networkx package which helped to remove the errors like zerodivision.
  2. Changes in community and centrality plots, newer version supports all the community detections like walk_trap, fastgreedy, etc.
  3. Changes in random seeds, color palletes etc.

Summary

This branch

Changes

The changes made in this PR are:

  1. Change 1
  2. Change 2

Checks

Notes

This fixes #

<other things, such as how to incorporate new changes>

szhorvat commented 8 months ago
  1. In new version Plot is based on networkx package which helped to remove the errors like zerodivision, This error was frequent in igraph implementation.

In what specific case do you see "division by zero" errors with igraph? If you found a problem with igraph, please open an issue and show a minimal example.

martinctc commented 8 months ago
  1. In new version Plot is based on networkx package which helped to remove the errors like zerodivision, This error was frequent in igraph implementation.

In what specific case do you see "division by zero" errors with igraph? If you found a problem with igraph, please open an issue and show a minimal example.

Thanks for flagging @szhorvat. I can confirm that the "division by zero" error occurs when an igraph plot is being generated in a graph with either no edge attributes (or potentially no edges), which happens to a graph when simplify() from igraph is run. This behaviour with simplify() (of seemingly removing too much edge data) did not seem to apply to the R version of igraph, and only the Python version of igraph. In PR #18, this issue was resolved by removing the simplify() step from the Python library, which allows us to preserve functionality without switching over to the networkx implementation. We will look into creating a minimal example to demonstrate this potential bug and submit to igraph. Thanks!

szhorvat commented 8 months ago

Thanks so much for tracking this down @martinctc! Can you have a look at this behaviour, @ntamas?

ntamas commented 8 months ago

Thanks so much for tracking this down @martinctc! Can you have a look at this behaviour, @ntamas?

Unfortunately I can't reproduce this with an empty graph (i.e. a graph with no edges) on my end. A SSCCE would be helpful to fix this on our side if it is indeed a bug.

martinctc commented 7 months ago

Thanks so much for tracking this down @martinctc! Can you have a look at this behaviour, @ntamas?

Unfortunately I can't reproduce this with an empty graph (i.e. a graph with no edges) on my end. A SSCCE would be helpful to fix this on our side if it is indeed a bug.

Ah okay. Will work on producing one and revert - thanks for checking @ntamas !