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

Add a reference to NXmpes from NXarpes #153

Merged
merged 4 commits into from
Jul 4, 2024
Merged

Add a reference to NXmpes from NXarpes #153

merged 4 commits into from
Jul 4, 2024

Conversation

domna
Copy link

@domna domna commented Feb 5, 2024

This is a proposal to refer NXmpes from NXarpes to avoid confusion for users of the definition.
This relates to #74 (i.e., whether it should refer to NXphotoemission or NXmpes).

It might also be a point for discussion whether NXarpes is a legacy version and newer experiments should be collected in NXmpes. Do we also support converting NXarpes to NXmpes/NXmpes_arpes?

@domna domna requested a review from sanbrock February 5, 2024 08:46
@domna
Copy link
Author

domna commented Feb 5, 2024

@rettigl @lukaspie @Tommaso-Pincelli What are your opinions on this?

@domna domna changed the title Add a reference to NXmpes to NXarpes Add a reference to NXmpes from NXarpes Feb 5, 2024
@lukaspie
Copy link
Collaborator

lukaspie commented Feb 5, 2024

It might also be a point for discussion whether NXarpes is a legacy version and newer experiments should be collected in NXmpes. Do we also support converting NXarpes to NXmpes/NXmpes_arpes?

I like the reference in general. I don't have a good overview whether NXarpes has been used or is still in use. If it is not even used anymore, I don't think such a conversion is necessary. If we want to do it, we can however provide a json file that maps from the original NXarpes to NXmpes through pynxtools. If nobody uses it, we could also suggest NIAC to make NXarpes deprecated if we want to have an even stronger statement.

@domna
Copy link
Author

domna commented Feb 5, 2024

It might also be a point for discussion whether NXarpes is a legacy version and newer experiments should be collected in NXmpes. Do we also support converting NXarpes to NXmpes/NXmpes_arpes?

I like the reference in general. I don't have a good overview whether NXarpes has been used or is still in use. If it is not even used anymore, I don't think such a conversion is necessary. If we want to do it, we can however provide a json file that maps from the original NXarpes to NXmpes through pynxtools. If nobody uses it, we could also suggest NIAC to make NXarpes deprecated if we want to have an even stronger statement.

Yeah I think whether it is used can only answer the NIAC but even then we might not know if it might be used somewhere unknowingly. So my suggestion is to keep it at least as legacy support as a definition.
I think making it deprecated and referring to NXmpes is a good approach. @sanbrock maybe you can already bring this to a discussion at one of the NIAC telcos? If they think it is not used we can prepare the deprecation and link to NXmpes with the September release.

@rettigl
Copy link
Collaborator

rettigl commented Feb 5, 2024

I also like the reference idea. For conversion (and deprication), we should check that we don't have any required items in NXmpes/NXphotoemission which are not available in NXarpes

Copy link
Collaborator

@lukaspie lukaspie left a comment

Choose a reason for hiding this comment

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

@domna I slightly modified the reference so that it references NXmpes AND NXmpes_arpes. Feel free to merge.

@domna
Copy link
Author

domna commented Jul 4, 2024

@domna I slightly modified the reference so that it references NXmpes AND NXmpes_arpes. Feel free to merge.

Thanks!

@domna domna merged commit f40319c into fairmat Jul 4, 2024
6 checks passed
@domna domna deleted the arpes-mpes-ref branch July 4, 2024 09:44
lukaspie added a commit that referenced this pull request Sep 23, 2024
* Add a reference to NXmpes to NXarpes

* Make reference an actual rst link

* Refer to `NXmpes_arpes`

* modify reference to NXmpes_arpes

---------

Co-authored-by: Lukas Pielsticker <[email protected]>
lukaspie pushed a commit that referenced this pull request Sep 23, 2024
* Add a reference to NXmpes to NXarpes

* Make reference an actual rst link

* Refer to `NXmpes_arpes`

* modify reference to NXmpes_arpes

---------

Co-authored-by: Lukas Pielsticker <[email protected]>
# Conflicts:
#	applications/NXarpes.nxdl.xml
#	applications/nyaml/NXarpes.yaml
lukaspie pushed a commit that referenced this pull request Sep 23, 2024
* Add a reference to NXmpes to NXarpes

* Make reference an actual rst link

* Refer to `NXmpes_arpes`

* modify reference to NXmpes_arpes

---------

Co-authored-by: Lukas Pielsticker <[email protected]>
# Conflicts:
#	applications/NXarpes.nxdl.xml
#	applications/nyaml/NXarpes.yaml
lukaspie pushed a commit that referenced this pull request Oct 16, 2024
* Add a reference to NXmpes to NXarpes

* Make reference an actual rst link

* Refer to `NXmpes_arpes`

* modify reference to NXmpes_arpes

---------

Co-authored-by: Lukas Pielsticker <[email protected]>
# Conflicts:
#	applications/NXarpes.nxdl.xml
#	applications/nyaml/NXarpes.yaml
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