Skip to content

Conversation

superstar54
Copy link
Member

@superstar54 superstar54 commented Nov 9, 2024

Reopen PR #560 .
In aiidalab-widgets-base/viewer.py, it creates a new viewer every time we select a new AiiDA node, and the viewer is not saved in the app, so we can not reuse it later. This has two drawbacks:

  • The performance is back. user needs to wait every time when selecting a new node.
  • One can not call the viewer later, e.g., for reset.

This PR stores the already added viewer and reuses it later.

This is also needed in the lazy-loading PR.

Copy link

codecov bot commented Nov 9, 2024

Codecov Report

Attention: Patch coverage is 28.57143% with 10 lines in your changes missing coverage. Please review.

Project coverage is 66.92%. Comparing base (2924582) to head (3de6ff9).
Report is 127 commits behind head on main.

Files with missing lines Patch % Lines
src/aiidalab_qe/app/result/__init__.py 28.57% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #921      +/-   ##
==========================================
- Coverage   67.06%   66.92%   -0.14%     
==========================================
  Files          51       51              
  Lines        4715     4726      +11     
==========================================
+ Hits         3162     3163       +1     
- Misses       1553     1563      +10     
Flag Coverage Δ
python-3.11 66.92% <28.57%> (-0.14%) ⬇️
python-3.9 66.95% <28.57%> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@superstar54 superstar54 changed the title Store node viewer Store already added node viewer and load it when needed Nov 9, 2024
@edan-bainglass
Copy link
Member

@superstar54 thank you for reopening this! It will greatly improve the usefulness of the lazy-loading work implemented in #802 w.r.t the results section. Will review ASAP!

Copy link
Member

@edan-bainglass edan-bainglass left a comment

Choose a reason for hiding this comment

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

LGTM! As stated, this will come in handy when node_views become MVCs. Good to not have to recreate them 🙂

@superstar54 superstar54 merged commit 5a55a5c into aiidalab:main Nov 11, 2024
8 checks passed
edan-bainglass added a commit to edan-bainglass/aiidalab-qe that referenced this pull request Nov 11, 2024
edan-bainglass added a commit to edan-bainglass/aiidalab-qe that referenced this pull request Nov 11, 2024
edan-bainglass added a commit that referenced this pull request Nov 11, 2024
Added a `refresh` parameter to allow bypassing the node view cache when triggering a manual update
edan-bainglass added a commit to edan-bainglass/aiidalab-qe that referenced this pull request Nov 11, 2024
edan-bainglass added a commit to aiidalab/aiidalab-widgets-base that referenced this pull request Feb 15, 2025
Moving node view caching mechanism implemented by @superstar54 in the QE app (aiidalab/aiidalab-qe#921) into `AiidaNodeViewWidget`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants