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

Humanized process status view #1048

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

edan-bainglass
Copy link
Member

@edan-bainglass edan-bainglass commented Jan 3, 2025

This PR implements a simpler process status tree.

The tree is

  • human readable
  • collapsible
  • linked with advanced node viewer
  • resettable (collapse all)
  • lazy loaded
  • animated (smooth collapse/expand - may require JS to compute heights - low priority)

TODO

  • Allow for dynamic growing total jobs count for restart workflows
  • Add tests

image


Current implementation also marks restart workflows with a *, including a description.

Screenshot 2025-01-10 092941

Future PRs will consider dynamic totals.

Component of #1042

@edan-bainglass edan-bainglass self-assigned this Jan 3, 2025
Copy link

codecov bot commented Jan 3, 2025

Codecov Report

Attention: Patch coverage is 91.44981% with 23 lines in your changes missing coverage. Please review.

Project coverage is 69.04%. Comparing base (416fb6f) to head (57bac8c).

Files with missing lines Patch % Lines
...c/aiidalab_qe/app/result/components/status/tree.py 93.01% 16 Missing ⚠️
...aiidalab_qe/app/result/components/status/status.py 88.88% 4 Missing ⚠️
src/aiidalab_qe/common/widgets.py 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1048      +/-   ##
==========================================
+ Coverage   67.99%   69.04%   +1.05%     
==========================================
  Files         113      114       +1     
  Lines        6734     6995     +261     
==========================================
+ Hits         4579     4830     +251     
- Misses       2155     2165      +10     
Flag Coverage Δ
python-3.11 69.04% <91.44%> (+1.05%) ⬆️
python-3.9 69.05% <91.07%> (+1.03%) ⬆️

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.

@AndresOrtegaGuerrero
Copy link
Member

Some issues you might find related , or that your PR will be addressing
#113
#87

@edan-bainglass
Copy link
Member Author

edan-bainglass commented Jan 3, 2025

Some issues you might find related , or that your PR will be addressing #113 #87

Thanks @AndresOrtegaGuerrero. Indeed related. Especially #87, the ideas of which I already had in mind. Though I was wondering if it should be generalized on the AiiDA level.

class MyWorkChain(WorkChain):
    _humanized_description = "Compute something"

    def get_human_readable_description(self):
        return self._humanized_description

Side question, we are currently on issue 1000+. Do you just happen to remember these ancient issues, or did you go looking? 😅 Asking for my own edification, so I can better find related issues.

@AndresOrtegaGuerrero
Copy link
Member

This was an old discussion , so i new Marnik mentioned this in the past, , but indeed there a need of cleaning old issues.

@edan-bainglass
Copy link
Member Author

This was an old discussion , so i new Marnik mentioned this in the past, , but indeed there a need of cleaning old issues.

No doubt! After the release, I plan to go through all issues, close what has not been automatically closed, and prep the agenda for the next release from all that are open.

@edan-bainglass edan-bainglass force-pushed the simplified-process-status-view branch 5 times, most recently from ececc68 to 86bbf58 Compare January 8, 2025 07:48
@edan-bainglass edan-bainglass force-pushed the simplified-process-status-view branch 2 times, most recently from 908c45e to 22dd242 Compare January 8, 2025 10:30
@AndresOrtegaGuerrero
Copy link
Member

Is this behaviour expected ,

Screen.Recording.2025-01-08.at.12.01.42.mov

If i go to Generate k-points (since it has a link) it sends me to the Advanced status view .. isnt a lil confusing ? or that is what is required ?

@AndresOrtegaGuerrero
Copy link
Member

I would suggest that in the "Humanized view: there is a link to the parameters from QuantumEspresso that arent visible in the summary , and that some users might want to easily see ,

image

@edan-bainglass
Copy link
Member Author

edan-bainglass commented Jan 8, 2025

Is this behaviour expected ,

Screen.Recording.2025-01-08.at.12.01.42.mov
If i go to Generate k-points (since it has a link) it sends me to the Advanced status view .. isnt a lil confusing ? or that is what is required ?

Intended behavior, but we since discussed changes that address all your comments. Will document here soon. Note that the idea is to implement a couple prior to the release, with the rest (requiring more work) punted down for now.

@edan-bainglass edan-bainglass force-pushed the simplified-process-status-view branch 2 times, most recently from 735919e to cab7963 Compare January 10, 2025 09:22
@edan-bainglass edan-bainglass force-pushed the simplified-process-status-view branch from 2faa2a5 to 765b66f Compare January 10, 2025 11:33
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.

2 participants