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.

Sign up for GitHub

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

Jump to bottom

Precompute the rank of a data node in all the scenario config DAGs. #2072

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

Precompute the rank of a data node in all the scenario config DAGs. #2072

wants to merge 9 commits into from

Conversation

jrobinAV
Copy link
Member

@jrobinAV jrobinAV commented Oct 16, 2024 •
edited by FredLL-Avaiga
Loading

resolves #2071

  • Each data node config now has a _rank attribute as a dictionary.
  • All the nested data node configs are updated when configuring a new scenario config. An entry is added in the DataNodeConfig._rank dictionary. The key is the scenario config id, and the value is the rank of the data node in the scenario config DAG.
  • Each data node now has a _rank attribute as a dictionary.
  • When a data node is instantiated, its _rank dictionary is copied from its config.

@jrobinAV jrobinAV mentioned this pull request Oct 16, 2024
Open
FlorianJacta
FlorianJacta previously approved these changes Oct 16, 2024
View reviewed changes
Copy link
Member

@FlorianJacta FlorianJacta left a comment

Choose a reason for hiding this comment

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

I agree with the principle. I let the others review the code 😄

@jrobinAV jrobinAV added Core Related to Taipy Core 🖰 GUI Related to GUI 🟨 Priority: Medium Not blocking but should be addressed labels Oct 16, 2024
@jrobinAV jrobinAV self-assigned this Oct 16, 2024
FredLL-Avaiga
FredLL-Avaiga previously approved these changes Oct 17, 2024
View reviewed changes
Copy link
Member

@FredLL-Avaiga FredLL-Avaiga left a comment

Choose a reason for hiding this comment

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

with comment

taipy/core/config/scenario_config.py Outdated Show resolved Hide resolved
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 17, 2024 •
edited
Loading

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
19257 16698 87% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
taipy/core/config/data_node_config.py 99% 🟢
taipy/core/config/scenario_config.py 98% 🟢
taipy/core/data/data_node.py 96% 🟢
taipy/gui_core/_adapters.py 46% 🟢
TOTAL 85% 🟢

updated for commit: 39276e5 by action🐍

@FredLL-Avaiga FredLL-Avaiga self-assigned this Oct 17, 2024
trgiangdo
trgiangdo reviewed Oct 19, 2024
View reviewed changes
Copy link
Member

@trgiangdo trgiangdo left a comment

Choose a reason for hiding this comment

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

Minor comments

taipy/core/data/data_node.py Outdated Show resolved Hide resolved
taipy/core/data/data_node.py Show resolved Hide resolved
@FredLL-Avaiga FredLL-Avaiga force-pushed the feature/#2071-rank-datanodes-in-selector branch from 1c622f9 to dbd2949 Compare October 22, 2024 08:26
trgiangdo
trgiangdo approved these changes Oct 31, 2024
View reviewed changes
Copy link
Member

@trgiangdo trgiangdo left a comment

Choose a reason for hiding this comment

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

LGTM

toan-quach
toan-quach approved these changes Oct 31, 2024
View reviewed changes
Copy link
Member

@toan-quach toan-quach left a comment

Choose a reason for hiding this comment

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

lgtm :D

@jrobinAV
Copy link
Member Author

Actually LNGTM.

I don't like it, and I'm having difficulty explaining why. Please give me some time to figure it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toan-quach toan-quach toan-quach approved these changes

@trgiangdo trgiangdo trgiangdo approved these changes

@FabienLelaquais FabienLelaquais Awaiting requested review from FabienLelaquais

@joaoandre-avaiga joaoandre-avaiga Awaiting requested review from joaoandre-avaiga

@FlorianJacta FlorianJacta Awaiting requested review from FlorianJacta

@FredLL-Avaiga FredLL-Avaiga Awaiting requested review from FredLL-Avaiga

Assignees

@jrobinAV jrobinAV

@FredLL-Avaiga FredLL-Avaiga

Labels
Core Related to Taipy Core 🖰 GUI Related to GUI 🟨 Priority: Medium Not blocking but should be addressed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Rank data nodes by topological order in the data node selector
5 participants
@jrobinAV @trgiangdo @FredLL-Avaiga @toan-quach @FlorianJacta