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

Add graph visualization feature #1976

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

satyyam11
Copy link

This pull request addresses issue #1592 by introducing a new graph visualization feature to the project.
The changes include a new graph_visualizer.py file with a function to visualize scenario configuration graphs using networkx and matplotlib.
Additionally, the config/init.py file was updated to load the configuration and call the visualization function. This enhancement provides a visual representation of nodes and edges in the configuration, making it easier to understand and debug complex scenarios.
Dependencies for networkx and matplotlib are required, which can be installed via pip. Please review and merge this PR to integrate the new feature into the project.

Copy link
Member

@jrobinAV jrobinAV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The requirements are not satisfied.

In the ScenarioConfig class, we want a new method named draw() that exports the scenario configuration graph as an image file.

The graph comprises the input DataNodeConfigs, the output DataNodeConfigs, and the TaskConfigs as nodes.
A directed edge exists from a DatanodeConfig node to a TaskConfig node if and only if the DatanodeConfig is a TaskConfig's input.
A directed edge exists from a TaskConfig node to a DatanodeConfig node if and only if the DatanodeConfig is a TaskConfig input.

If a new package is required, it must be made optional. The draw function should check the presence of the package before using it. If the package is not installed, the draw function should log a warning message and return doing nothing else.

Copy link
Member

@jrobinAV jrobinAV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several remarks:

  • The main code should not be committed.
  • The ScenarioConfig class already exists. You should use it.
  • The creation of the graph is not integrated with Taipy. The graph already exists as a set of task configs. It must be converted into a networkx DAG. A dedicated method could be exposed in the ScenarioConfig class for building the dag as a nx object.
  • The feature should be generic. In particular, the size of the graph may vary a lot.
  • Many unit tests are expected in this PR. Several use cases must be tested with various graph sizes and shapes.
  • No print is allowed. Please use a Taipy Logger.
  • The pictures should not be committed.

@@ -0,0 +1,46 @@
import matplotlib.pyplot as plt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be an optional dependency. Please verify it is installed before using it.

import networkx as nx


class ScenarioConfig:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a Scenario Config class that already contains a DAG. It must be re-used.
The purpose is to add a draw method to this class.

@jrobinAV jrobinAV added Core Related to Taipy Core ⚙️Configuration Related to Taipy Configuration Core: ⚙️ Configuration 🟩 Priority: Low Low priority and doesn't need to be rushed 🟨 Priority: Medium Not blocking but should be addressed hacktoberfest hacktoberfest issues hacktoberfest - 100💎 Issues rewarded by 100 points and removed ⚙️Configuration Related to Taipy Configuration 🟩 Priority: Low Low priority and doesn't need to be rushed labels Oct 17, 2024
@jrobinAV jrobinAV self-assigned this Oct 17, 2024
@jrobinAV
Copy link
Member

@satyyam11 any news? Do my comments make sense to you?

@Manideep-Maddileti
Copy link

i want to work on this issue please assig to me @jrobinAV

@jrobinAV
Copy link
Member

@Manideep-Maddileti This is a Pull request. Please feel free to contribute by reviewing the code, and add any comment.

If you want, you can also be assigned to the related issue #1592 and propose your own Pull Request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core: ⚙️ Configuration Core Related to Taipy Core hacktoberfest - 100💎 Issues rewarded by 100 points hacktoberfest hacktoberfest issues 🟨 Priority: Medium Not blocking but should be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants