Skip to content

Add new application definition for PEEM experiments#65

Closed
PeterC-DLS wants to merge 8 commits intoFAIRmat-NFDI:mpes-refactorfrom
DiamondLightSource:mpes-peem
Closed

Add new application definition for PEEM experiments#65
PeterC-DLS wants to merge 8 commits intoFAIRmat-NFDI:mpes-refactorfrom
DiamondLightSource:mpes-peem

Conversation

@PeterC-DLS
Copy link

This extends NXmpes and adds some new groups and fields and also adjusts some existing fields' classes

@domna domna mentioned this pull request Sep 13, 2023
Copy link

@domna domna left a comment

Choose a reason for hiding this comment

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

Hey @PeterC-DLS,

thank you for your proposal. I left some comments in the yaml file.

domna and others added 4 commits September 13, 2023 18:08
This extends NXmpes and adds some new groups and fields and also adjusts some existing fields' classes
Move YAML to nyaml directory, tweak PEEM definition according to feedback, push some
changes to base MPES definition and add some units
@domna
Copy link

domna commented Oct 11, 2023

Hey @PeterC-DLS,

I just wanted to let you know that we started to add an additional level on top of NXmpes: NXphotoemission. See #74 for further details.
Our idea was that NXmpes should contain an energy axis since we say it is a spectroscopy and that we have other techniques (like PEEM) that do not necessarily have an energy axis. Therefore, we moved all of the fields of NXmpes into NXphotoemission and you might just use NXphotoemission as a drop-in replacement for NXmpes in the future. We will, however, may move some fields from NXphotoemission to NXmpes if we think they rather are spectroscopic properties (but I think this will not be of much concern for NXpeem either).

Other than this: Can I help you with anything to finalise this PR? Are there currently any open points/blockers for you?

@PeterC-DLS
Copy link
Author

Thanks for the update and pointer. Your PR sounds fine to me.

@domna
Copy link

domna commented Oct 11, 2023

Thanks for the update and pointer. Your PR sounds fine to me.

Sure and thanks.
I was also wondering about the status of this NXpeem PR. Is there anything you still like to add/change? Are there things which need discussion?

@PeterC-DLS
Copy link
Author

I will update and rebase this PR once you have finalized or merged your refactor of NXmpes to NXphotoemission.

@domna domna mentioned this pull request Oct 24, 2023
4 tasks
@domna domna added the mpes label Dec 4, 2023
@lukaspie lukaspie deleted the branch FAIRmat-NFDI:mpes-refactor January 15, 2024 16:20
@lukaspie lukaspie closed this Jan 15, 2024
@domna
Copy link

domna commented Jan 15, 2024

@PeterC-DLS I think this auto closed due to our merge of mpes-refactor, but feel free to set the target to fairmat and re-open this PR.

We did not yet add the top-level class NXphotoemission, because we did not want to overcomplicate the definitions for our workshop next week. But it will be merged afterwards (see #74). Feel free to move forward with this or wait until we do the changes. Whatever is best for you.

@PeterC-DLS
Copy link
Author

PeterC-DLS commented Jan 15, 2024

I have just rebase this to the old branch! But it's not a problem for me to rebase as and when NXphotoemission is ready.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants