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

use table_widget #1068

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

Conversation

superstar54
Copy link
Member

@superstar54 superstar54 commented Jan 9, 2025

Use the new Table wdiget which supports editing, filtering features.

Supports:

  • to edit label and description of the job
  • custom columns by show/hide columns
  • a quick search bar to find jobs by any visible field.
  • add exit_status and exit_message
new-calculation-history-page.mp4

Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 62.88660% with 36 lines in your changes missing coverage. Please review.

Project coverage is 68.96%. Comparing base (d89548a) to head (fcd8467).

Files with missing lines Patch % Lines
src/aiidalab_qe/app/utils/search_jobs.py 62.88% 36 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1068      +/-   ##
==========================================
+ Coverage   67.98%   68.96%   +0.98%     
==========================================
  Files         113      113              
  Lines        6734     6764      +30     
==========================================
+ Hits         4578     4665      +87     
+ Misses       2156     2099      -57     
Flag Coverage Δ
python-3.11 68.96% <62.88%> (+0.98%) ⬆️
python-3.9 68.98% <62.88%> (+0.98%) ⬆️

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.

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.

Very nice @superstar54! Please see my comments.

calculation_history.ipynb Outdated Show resolved Hide resolved
Comment on lines +8 to 13
STATE_ICONS = {
"running": "⏳",
"finished": "✅",
"excepted": "⚠️",
"killed": "❌",
}
Copy link
Member

Choose a reason for hiding this comment

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

I use mostly the same icons in the process tree. Best we agree on an icon set and place it in the common module.

src/aiidalab_qe/app/utils/search_jobs.py Outdated Show resolved Hide resolved
src/aiidalab_qe/app/utils/search_jobs.py Outdated Show resolved Hide resolved
src/aiidalab_qe/app/utils/search_jobs.py Outdated Show resolved Hide resolved
src/aiidalab_qe/app/utils/search_jobs.py Outdated Show resolved Hide resolved
src/aiidalab_qe/app/utils/search_jobs.py Outdated Show resolved Hide resolved
src/aiidalab_qe/app/utils/search_jobs.py Outdated Show resolved Hide resolved
@AndresOrtegaGuerrero
Copy link
Member

Nice @superstar54 , I was wondering , can #1045 be address in the PR ?

- sentence case
- add `children`
- format lines
@superstar54
Copy link
Member Author

I was wondering , can #1045 be address in the PR ?

Yes, thanks for pointing out this. I added this feature as shown below:

Screenshot from 2025-01-09 17-04-19

@superstar54
Copy link
Member Author

superstar54 commented Jan 9, 2025

Hi @edan-bainglass , I resolved all your comments except the icons. We can unify the icons later. Please review.

@edan-bainglass
Copy link
Member

edan-bainglass commented Jan 9, 2025

I was wondering , can #1045 be address in the PR ?

Yes, thanks for pointing out this. I added this feature as shown below:

Remember to close #1045 if done 🙏

@edan-bainglass
Copy link
Member

edan-bainglass commented Jan 9, 2025

Screenshot 2025-01-09 184941

  1. Header is too much. Keep it as it was before, but sentence case
  2. Page guide button is perfect!
  3. Remove the border from everything. It's not adding anything. If you disagree, make sure to add padding to anything that has a border, otherwise, it's looks too cramped.
  4. I have plenty of jobs. Why none showing up?

@superstar54
Copy link
Member Author

I have plenty of jobs. Why none showing up?

How do you install the table_widget from the repo or use pip? If from repo, you also need to run npm install & npm run build

Hi @edan-bainglass , I have resolved all other issues.

@edan-bainglass
Copy link
Member

I have plenty of jobs. Why none showing up?

How do you install the table_widget from the repo or use pip? If from repo, you also need to run npm install & npm run build

I'm installing from pypi. I've upgraded to 0.0.2. Still no table showing!

Hi @edan-bainglass , I have resolved all other issues.

Screenshot 2025-01-10 074243

Simple header (as we do for all notebook except the app). h1 tag, no CSS class. Maybe add some bottom margin if you want.
We can discuss next week if we want all notebook to carry the banner, but that should be for the next release (or minor release).

Also, I really think you should remove the borders. I also removed the margin from the table component. It is much cleaner without it.

@edan-bainglass
Copy link
Member

Screenshot 2025-01-10 074243

Simple header (as we do for all notebook except the app). h1 tag, no CSS class. Maybe add some bottom margin if you want. We can discuss next week if we want all notebook to carry the banner, but that should be for the next release (or minor release).

Also, I really think you should remove the borders. I also removed the margin from the table component. It is much cleaner without it.

Alternatively, if we do want borders

Screenshot 2025-01-10 075616

Styles

  • Heading: h1 with 15px bottom margin
  • boxes:
    • border: 1px solid lightgray
    • padding: 0.5em
  • properties filter box
    • same as above, but also margin: 5px

@superstar54
Copy link
Member Author

We can discuss next week if we want all notebook to carry the banner, but that should be for the next release (or minor release).

agree, we should consider to uniform the page header and footer for all notebooks.

I'm installing from pypi. I've upgraded to 0.0.2. Still no table showing!

Any error in the browser console?

Thanks for for the style suggestion, I have implemented the styles.

@edan-bainglass
Copy link
Member

We can discuss next week if we want all notebook to carry the banner, but that should be for the next release (or minor release).

agree, we should consider to uniform the page header and footer for all notebooks.

I'm installing from pypi. I've upgraded to 0.0.2. Still no table showing!

Any error in the browser console?

Thanks for for the style suggestion, I have implemented the styles.

Not sure why, but it works now. I can see my jobs. Styles look great! However, I am getting an error.

Filtered by PDOS, then by running job state, and got

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
/opt/conda/lib/python3.9/site-packages/ipywidgets/widgets/widget.py in _handle_msg(self, msg)
    762                 if 'buffer_paths' in data:
    763                     _put_buffers(state, data['buffer_paths'], msg['buffers'])
--> 764                 self.set_state(state)
    765 
    766         # Handle a state request.

/opt/conda/lib/python3.9/site-packages/ipywidgets/widgets/widget.py in set_state(self, sync_data)
    631                     from_json = self.trait_metadata(name, 'from_json',
    632                                                     self._trait_from_json)
--> 633                     self.set_trait(name, from_json(sync_data[name], self))
    634 
    635     def send(self, content, buffers=None):

/opt/conda/lib/python3.9/contextlib.py in __exit__(self, typ, value, traceback)
    124         if typ is None:
    125             try:
--> 126                 next(self.gen)
    127             except StopIteration:
    128                 return False

~/.local/lib/python3.9/site-packages/traitlets/traitlets.py in hold_trait_notifications(self)
   1500                 for changes in cache.values():
   1501                     for change in changes:
-> 1502                         self.notify_change(change)
   1503 
   1504     def _notify_trait(self, name, old_value, new_value):

/opt/conda/lib/python3.9/site-packages/ipywidgets/widgets/widget.py in notify_change(self, change)
    692                 # Send new state to front-end
    693                 self.send_state(key=name)
--> 694         super(Widget, self).notify_change(change)
    695 
    696     def __repr__(self):

~/.local/lib/python3.9/site-packages/traitlets/traitlets.py in notify_change(self, change)
   1515     def notify_change(self, change):
   1516         """Notify observers of a change event"""
-> 1517         return self._notify_observers(change)
   1518 
   1519     def _notify_observers(self, event):

~/.local/lib/python3.9/site-packages/traitlets/traitlets.py in _notify_observers(self, event)
   1562                 c = getattr(self, c.name)
   1563 
-> 1564             c(event)
   1565 
   1566     def _add_notifiers(self, handler, name, type):

/opt/conda/lib/python3.9/site-packages/ipywidgets/widgets/widget_selection.py in _propagate_index(self, change)
    233             self.label = label
    234         if self.value is not value:
--> 235             self.value = value
    236 
    237     @validate('value')

~/.local/lib/python3.9/site-packages/traitlets/traitlets.py in __set__(self, obj, value)
    730             raise TraitError('The "%s" trait is read-only.' % self.name)
    731         else:
--> 732             self.set(obj, value)
    733 
    734     def _validate(self, obj, value):

~/.local/lib/python3.9/site-packages/traitlets/traitlets.py in set(self, obj, value)
    719             # we explicitly compare silent to True just in case the equality
    720             # comparison above returns something other than True/False
--> 721             obj._notify_trait(self.name, old_value, new_value)
    722 
    723     def __set__(self, obj, value):

~/.local/lib/python3.9/site-packages/traitlets/traitlets.py in _notify_trait(self, name, old_value, new_value)
   1503 
   1504     def _notify_trait(self, name, old_value, new_value):
-> 1505         self.notify_change(
   1506             Bunch(
   1507                 name=name,

/opt/conda/lib/python3.9/site-packages/ipywidgets/widgets/widget.py in notify_change(self, change)
    692                 # Send new state to front-end
    693                 self.send_state(key=name)
--> 694         super(Widget, self).notify_change(change)
    695 
    696     def __repr__(self):

~/.local/lib/python3.9/site-packages/traitlets/traitlets.py in notify_change(self, change)
   1515     def notify_change(self, change):
   1516         """Notify observers of a change event"""
-> 1517         return self._notify_observers(change)
   1518 
   1519     def _notify_observers(self, event):

~/.local/lib/python3.9/site-packages/traitlets/traitlets.py in _notify_observers(self, event)
   1562                 c = getattr(self, c.name)
   1563 
-> 1564             c(event)
   1565 
   1566     def _add_notifiers(self, handler, name, type):

~/apps/quantum-espresso/src/aiidalab_qe/app/utils/search_jobs.py in apply_filters(self, _)
    335                 & (filtered_df["ctime"] <= end_time)
    336             ]
--> 337         self.update_table_value(filtered_df)
    338 
    339     def update_table_visibility(self, _):

~/apps/quantum-espresso/src/aiidalab_qe/app/utils/search_jobs.py in update_table_value(self, display_df)
    291         # Adjust the ID column based on the toggle state
    292         if self.toggle_id_format.value == "PK":
--> 293             display_df = display_df.rename(columns={"PK_with_link": "ID"}).drop(
    294                 columns=["UUID_with_link"]
    295             )

~/.local/lib/python3.9/site-packages/pandas/core/frame.py in drop(self, labels, axis, index, columns, level, inplace, errors)
   5579                 weight  1.0     0.8
   5580         """
-> 5581         return super().drop(
   5582             labels=labels,
   5583             axis=axis,

~/.local/lib/python3.9/site-packages/pandas/core/generic.py in drop(self, labels, axis, index, columns, level, inplace, errors)
   4786         for axis, labels in axes.items():
   4787             if labels is not None:
-> 4788                 obj = obj._drop_axis(labels, axis, level=level, errors=errors)
   4789 
   4790         if inplace:

~/.local/lib/python3.9/site-packages/pandas/core/generic.py in _drop_axis(self, labels, axis, level, errors, only_slice)
   4828                 new_axis = axis.drop(labels, level=level, errors=errors)
   4829             else:
-> 4830                 new_axis = axis.drop(labels, errors=errors)
   4831             indexer = axis.get_indexer(new_axis)
   4832 

~/.local/lib/python3.9/site-packages/pandas/core/indexes/base.py in drop(self, labels, errors)
   7068         if mask.any():
   7069             if errors != "ignore":
-> 7070                 raise KeyError(f"{labels[mask].tolist()} not found in axis")
   7071             indexer = indexer[~mask]
   7072         return self.delete(indexer)

KeyError: "['UUID_with_link'] not found in axis"

@superstar54
Copy link
Member Author

Filtered by PDOS, then by running job state, and got

This is because no result is found, and the pd.dataframe is empty. I fixed it, and it can handle empty data. Besides, I also dropped the usage of pandas because I remember @danielhollas mentioned that he wanted to drop pandas in the AWB. The performance seems not affected.

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.

Enhance Calculation History with Workflow Error Details Make the job list page pretty
3 participants