Skip to content

Conversation

@lukaspie
Copy link
Contributor

@lukaspie lukaspie commented Sep 24, 2024

These introduces new base classes that are needed for more than one of these other PRs:
#1422
#1423
#1424 (merged already)
#1425 (votes passed, acceptance but not merged yet as of 2025/06/16)

Bundling for easier review.

Update 2025/06/16, commit 3b597b8
Most changes that are here proposed have been superseeded by other PRs, for some, decisions have already been taken, that make the changes here obsolete. This PR needs to be updated to implement the following, almost all changes here are obsolete, the PR should be revisited when #1422, #1423, and #1425 have been merged:

base classes

contributed_definitions

atomprobe-tc and others added 30 commits July 4, 2024 15:36
…or_circuits

Cleaned the too many base classes for circuits and cleaned base classes for describing a computer
Make geometries and sourceTYPE recommended in NXxps
…NXms with NXmicrostructure to avoid confusing the symbol convention with mass spectrometry as the abbreviation NXms may suggest
…con and eventually make this NXmicrostructure
@lukaspie lukaspie marked this pull request as ready for review September 24, 2024 10:59
@prjemian
Copy link
Contributor

Any new classes, whether intended to be a base class or an application definition, must go into contributed definitions until they are ratified by the NIAC.

Copy link
Contributor

@prjemian prjemian left a comment

Choose a reason for hiding this comment

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

Any new classes must be add to contributed_definitions/, not directly to base_classes/ or application_definnitions/.

@lukaspie lukaspie linked an issue Sep 29, 2024 that may be closed by this pull request
This was referenced Sep 29, 2024
@lukaspie lukaspie marked this pull request as draft November 27, 2024 21:55
@mkuehbach
Copy link
Contributor

See changes made 2025/06/16 in the top-level description of the PR

@lukaspie
Copy link
Contributor Author

@mkuehbach I think eventually, when we have the four PRs that are mentioned right at the top here merged, we can just close this PR. I don't think that there is anything here that was not discussed in the other PRs. But we can double check afterwards.

My suggestion: we leave this PR open until the others have been merged, do a rebase afterwards, and if there is nothing left to propose, only then we close here.

Note that NXactivity has already been accepted and merged and is used within NXmpes.

@mkuehbach
Copy link
Contributor

@lukaspie thanks for adding here the context, this mirrors with my read-through in the morning. Yeah, this one should be revisited when #1422 and #1423 have been merged

@lukaspie lukaspie force-pushed the fairmat-2024-common-base-classes branch from 5595da9 to 8d08c37 Compare August 27, 2025 16:30
@lukaspie
Copy link
Contributor Author

@mkuehbach I went back and merged the main branch into this one. I suspect the remaining new classes are not needed anymore, right? Then I would close here.

@mkuehbach
Copy link
Contributor

mkuehbach commented Aug 27, 2025

@lukaspie thank you, correct:

  • NXchamber was refactored via NXcomponent in both NXem and NXapm
  • NXsingle_crystal is obsolete, NXphase and NXmicrostructure are stronger alternatives
  • NXroi is obsolete, replaced and accepted via NXroi_process
  • NXstage_lab is obsolete, using NXmanipulator now throughout

Yes, this PR can be closed as resolved by previously voted and merge FAIRmat-2024 PRs

@lukaspie lukaspie closed this Aug 28, 2025
@lukaspie lukaspie deleted the fairmat-2024-common-base-classes branch August 28, 2025 06:37
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.

new common base classes

8 participants