8000 Update NeXus search app in pynxtools by GinzburgLev · Pull Request #580 · FAIRmat-NFDI/pynxtools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Update NeXus search app in pynxtools #580

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
Merged
merged 4 commits into from
Mar 12, 2025
Merged

Update NeXus search app in pynxtools #580

merged 4 commits into from
Mar 12, 2025

Conversation

GinzburgLev
Copy link
Contributor
@GinzburgLev GinzburgLev commented Mar 7, 2025 •
edited
Loading

A generic app for search/statistics visualization for NXsensor_scan and inheriting classes (currently NXiv_temp, NXspm, NXstm, NXafm, NXsts); also will work for NOMAD CAMELS entries (most likely, version>=1.8.0).
Please let me know if:

  • NXspm/sts/stm/afm etc should be removed from the filter, as they have a dedicated app with more suitable options
  • NOMAD CAMELS should be mentioned more/less/not at all (currently only menu/protocols(NOMAD CAMELS and protocol column, which is not shown by default) have filters that work only for them)
  • Any functionality / descriptions shall be added or improved
  • Any way to replace explicit list of classes inheriting from NXsensor_scan with something more automated exists

For tests, files from corresponding example uploads will work. Test CAMELS files can be found here: https://box.hu-berlin.de/d/2812ff0b8104478ba160/ (just put any of them in an upload without any extra files/schemas)

@GinzburgLev GinzburgLev requested a review from sanbrock March 7, 2025 16:48
sanbrock
sanbrock reviewed Mar 9, 2025
View reviewed changes
@RubelMozumder
Copy link
Collaborator

You can extend the nexus_app instead of having another app in pynxtools. I think that is what you wanted to do.

@GinzburgLev GinzburgLev changed the title Add new app for NXsensor_scan and inheriting classes (this will also work for NOMAD CAMELS output files) Update NeXus search app in pynxtools Mar 11, 2025
@GinzburgLev
Copy link
Contributor Author
GinzburgLev commented Mar 11, 2025 •
edited
Loading

Now this is not a new app, but an update for existing NeXus app (PR name changed accordingly). I tried to include the comments from today discussion. Several remaining issues that I was not able to fix:

  1. For every widget/menu item except entry_type, if I select more than one option, the search query is A OR B. For entry_type, it is A AND B. I do not know what this depends on.
  2. end time has not been added - currently for start time I use data.ENTRY[*].start_time and not data.ENTRY[*].start_time__field due to definition for NXsensor_scan not yet updated for pynxtools (still wrong format for the field). There is no corresponding field for end_time. Would be easy to change after the updated definition has been made available in pynxtools.
  3. Sorting along most columns (such as Start Time) is not possible - according to Lauri, sorting can only be done along scalar fields, so anything within data.ENTRY can not be used for sorting.
  4. Appearance of the dashboard on screens with different resolution: currently looks OK on a large screen (3840x2160, scale=150%) - one line of widgets, multiple entries are visible in the search results. On a reasonably small screen (1920x1080, scale=125%) just having a periodic table and no other widgets is too much - one has to scroll down to see search results. I am not sure what kind of compromise I should take.

Screenshot from 2025-03-11 16-08-42
Screenshot from 2025-03-11 16-08-55
Screenshot from 2025-03-11 16-09-25

@GinzburgLev
Copy link
Contributor Author

Updated layouts for smaller screen resolutions now. It still does not look very good at 1368x768, scale 100% (there I really can not do much, but also such screens are not that common for modern PCs). It is reasonable for everything larger, in my opinion. Attached are screenshots for 1920x1080, 1600x900 and 1368x768, all at 100% scale. For larger screens, I did not change anything from before.

Screenshot from 2025-03-12 10-25-33
Screenshot from 2025-03-12 10-26-07
Screenshot from 2025-03-12 10-26-55

Regarding AND / OR condition for multiple choice in Menu / Widget - a known issue of nomad, here is a relevant issue: https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/issues/2278

I do not think I can improve much more at the moment.

sanbrock
sanbrock approved these changes Mar 12, 2025
View reviewed changes
Copy link
Collaborator
@sanbrock sanbrock left a comment

Choose a reason for hiding this comment

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

LGTM

@rettigl
Copy link
Collaborator
rettigl commented Mar 12, 2025

@GinzburgLev So the histogram of nexus terms in the menu does work for you? Did you use some special branch of Nomad for this? Maybe I did something wrong in my tests...

@GinzburgLev
Copy link
Contributor Author
GinzburgLev commented Mar 12, 2025 •
edited
Loading

@GinzburgLev So the histogram of nexus terms in the menu does work for you? Did you use some special branch of Nomad for this? Maybe I did something wrong in my tests...

@rettigl I did not use anything special. Maybe this recent fix is relevant? https://gitlab.mpcdf.mpg.de/nomad-lab/nomad-FAIR/-/merge_requests/2367

@GinzburgLev GinzburgLev merged commit aa77148 into master Mar 12, 2025
17 checks passed
@GinzburgLev GinzburgLev deleted the search_app_for_NXsensor_scan_and_inhereting_classes branch March 12, 2025 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sanbrock sanbrock sanbrock approved these changes

@RubelMozumder RubelMozumder Awaiting requested review from RubelMozumder

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@GinzburgLev @RubelMozumder @rettigl @sanbrock
0