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.

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

Fairmat 2024: new fields for experiment description in NXentry and NXsubentry #1412

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

Fairmat 2024: new fields for experiment description in NXentry and NXsubentry #1412

wants to merge 17 commits into from

Conversation

lukaspie
Copy link
Contributor

@lukaspie lukaspie commented Sep 23, 2024 •
edited
Loading

Here, several new fields are added to NXentry, to give a more detailed of where (experiment_{location,facility,institution} ) and when (experiment_{start_date,end_date}) the experiment was performed.

@lukaspie lukaspie marked this pull request as draft September 23, 2024 15:06
@lukaspie lukaspie force-pushed the fairmat-2024-nxentry branch from ff097f7 to e0b24a6 Compare September 23, 2024 15:13
@lukaspie lukaspie marked this pull request as ready for review September 23, 2024 15:15
This was referenced Sep 29, 2024
Open
Closed
This was linked to issues Sep 29, 2024
NXentry #1450
Open
NXsubentry #1460
Closed
@woutdenolf
Copy link
Contributor

Do we know what these are identifiers of? There is an issue about it: #1043

@phyy-nx
Copy link
Contributor

phyy-nx commented Sep 29, 2024

Instead of this, add PID to NXobject as an attribute.

@woutdenolf
Copy link
Contributor

Will a group ever need more than one PID?

@lukaspie lukaspie changed the title Fairmat 2024: introduce NXidentfier, use in NXentry and NXsubentry Fairmat 2024: introduce NXidentifier, use in NXentry and NXsubentry Oct 7, 2024
@lukaspie lukaspie changed the title Fairmat 2024: introduce NXidentifier, use in NXentry and NXsubentry Fairmat 2024: new fields for experiment description in NXentry and NXsubentry Oct 16, 2024
@lukaspie
Copy link
Contributor Author

lukaspie commented Dec 6, 2024

Note that previous versions of this PR contained changes to the *_identifier fields in NXentry to make use of the then-proposed new base class NXidentifier. Currently, all discussion of identifiers has been removed.

@phyy-nx @sanbrock since there has anyway been no vote, I would suggest we wait with this PR until the discussion around #1486 has been settled and then make the necessary changes here on top afterwards.

This was referenced Dec 6, 2024
Open
Open
@lukaspie lukaspie force-pushed the fairmat-2024-nxentry branch from 22a6bf9 to 9e877ad Compare January 15, 2025 11:54
@phyy-nx
Copy link
Contributor

phyy-nx commented Feb 4, 2025

@lukaspie since #1486 has been merged is this one ready for discussion again?

@lukaspie
Copy link
Contributor Author

lukaspie commented Feb 5, 2025

@lukaspie since #1486 has been merged is this one ready for discussion again?

We were thinking about replacing the existing identifiers with fields starting with identifierNAME. For example, adding an identifier_entry and deprecating entry_identifier. With the idea that all identifiers in NeXus should follow this general idea of starting with identifier.... I haven't implemented this yet though. Would you think that this is a good way forward? If so, I can implement it and this PR would be ready to be discussed. All other changes are ready anyway.

@phyy-nx
Copy link
Contributor

phyy-nx commented Feb 5, 2025

Seems reasonable to me.

@lukaspie lukaspie force-pushed the fairmat-2024-nxentry branch 2 times, most recently from 23ce833 to 0097b28 Compare February 6, 2025 15:20
@lukaspie
Copy link
Contributor Author

lukaspie commented Feb 6, 2025

Seems reasonable to me.

I made the necessary changes and deprecated the ..._identifier fields. Probably warrants some discussion whether all of these should be kept in the new identifier_... format, but this PR should be ready to be discussed now.

domna and others added 8 commits February 6, 2025 16:22
@domna @lukaspie
Readds mpes relevant changes from 4b064d9
4bcf506
@RubelMozumder @lukaspie
Regeneration of the nexus file for fixing the changes coming from old…
4154bf7 
… version of yaml.

Removing unintensional comments
@domna @lukaspie
Convenience functions for nyaml ⇄ nxdl and gh pages build (#67)
137f0eb 
* Add appdef/bc yaml files to gitignore

* Improve Makefile for nxdl build

* Tricking make to avoid circular dependency

* Generalization and cleanup

* Change chmod 755 to 644 for NXentry

* Reintroduce for-loop for make nyaml

* Adds push to github pages

* Set correct docs dir

* Prepare building

* Change id to name

* Build and deploy

* Push pages to fairmat-docs

* Adds clean flag

* Use docs folder

* Target fairmat branch to rebuild live-docs

* Removes old workflows

* Updates color and logo

* Optimise styling

* Add logo padding

* Run ci on pr

* Adds cleanup ci

* Removes yaml files from .gitignore
# Conflicts:
#	.gitignore
#	Makefile
#	manual/source/_static/to_alabaster.css
#	manual/source/img/FAIRmat_new.png
@lukaspie
use NXidentifier in NXentry
1187cac 
# Conflicts:
#	base_classes/NXsubentry.nxdl.xml
#	base_classes/nyaml/NXentry.yaml
#	base_classes/nyaml/NXsubentry.yaml
#	contributed_definitions/NXsts.nxdl.xml
#	contributed_definitions/nyaml/NXsensor_scan.yaml
#	contributed_definitions/nyaml/NXsts.yaml
#	contributed_definitions/nyaml/NXtransmission.yaml
@lukaspie
format base classes and applications in accordance to existing NIAC v…
b869d50 
…ersion

# Conflicts:
#	applications/NXarpes.nxdl.xml
#	applications/nyaml/NXarpes.yaml
#	base_classes/NXaperture.nxdl.xml
#	base_classes/NXbeam.nxdl.xml
#	base_classes/NXdata.nxdl.xml
#	base_classes/NXdetector.nxdl.xml
#	base_classes/NXenvironment.nxdl.xml
#	base_classes/NXinstrument.nxdl.xml
#	base_classes/NXmonochromator.nxdl.xml
#	base_classes/NXroot.nxdl.xml
#	base_classes/NXsample.nxdl.xml
#	base_classes/NXsample_component.nxdl.xml
#	base_classes/NXsensor.nxdl.xml
#	base_classes/NXsource.nxdl.xml
#	base_classes/NXsubentry.nxdl.xml
#	base_classes/NXtransformations.nxdl.xml
#	base_classes/NXuser.nxdl.xml
#	base_classes/nyaml/NXaperture.yaml
#	base_classes/nyaml/NXbeam.yaml
#	base_classes/nyaml/NXdata.yaml
#	base_classes/nyaml/NXdetector.yaml
#	base_classes/nyaml/NXentry.yaml
#	base_classes/nyaml/NXenvironment.yaml
#	base_classes/nyaml/NXinstrument.yaml
#	base_classes/nyaml/NXmonochromator.yaml
#	base_classes/nyaml/NXprocess.yaml
#	base_classes/nyaml/NXroot.yaml
#	base_classes/nyaml/NXsample.yaml
#	base_classes/nyaml/NXsample_component.yaml
#	base_classes/nyaml/NXsensor.yaml
#	base_classes/nyaml/NXsource.yaml
#	base_classes/nyaml/NXsubentry.yaml
#	base_classes/nyaml/NXtransformations.yaml
#	base_classes/nyaml/NXuser.yaml
@lukaspie
pull out modifications for fairmat-2024-nxentry
23145bb
@lukaspie
revert unintentional changes from cherry-pick
b9c8947
@lukaspie
bring in changes on NXsubentry
3fc715e
@lukaspie lukaspie force-pushed the fairmat-2024-nxentry branch from 0097b28 to 2fc892b Compare February 6, 2025 15:22
@lukaspie lukaspie mentioned this pull request Feb 26, 2025
Open
lukaspie added a commit to FAIRmat-NFDI/nexus_definitions that referenced this pull request Feb 26, 2025
@lukaspie
update NXentry and NXsubentry from nexusformat#1412
f8ba778
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NXentry NXsubentry
5 participants
@lukaspie @woutdenolf @phyy-nx @domna @RubelMozumder