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

#243 Sprint22 Add stigmation correctors, removed unnecessary EM-Base Classes, opened NXcomponent for other techniques #250

Closed
wants to merge 28 commits into from

Conversation

mkuehbach
Copy link
Collaborator

No description provided.

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.

Mostly small questions, but I do have an issue as to how these new base classes are supposed to fit within the NXem ecosystem. Most of the new base classes are not used in NXem (or in the base classes that are specialized within NXem), so how are they supposed to be used in the appdef? As example, searches for NXmonochromator_em and NXcorrector_ax don't show them being cited anywhere else.

EDIT: I see that #251 adds most of them to NXem. Should anyway be checked that they are all cited?

contributed_definitions/nyaml/NXmonochromator_em.yaml Outdated Show resolved Hide resolved
contributed_definitions/nyaml/NXbiprism_em.yaml Outdated Show resolved Hide resolved
contributed_definitions/nyaml/NXcomponent_em.yaml Outdated Show resolved Hide resolved
contributed_definitions/nyaml/NXcomponent_em.yaml Outdated Show resolved Hide resolved
contributed_definitions/nyaml/NXcomponent_em.yaml Outdated Show resolved Hide resolved
contributed_definitions/nyaml/NXcorrector_ax.yaml Outdated Show resolved Hide resolved
contributed_definitions/nyaml/NXcorrector_ax.yaml Outdated Show resolved Hide resolved
contributed_definitions/nyaml/NXcorrector_cs.yaml Outdated Show resolved Hide resolved
… NXcomponent for technical components through removing electron microscopy specificity
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.

lgtm

@mkuehbach mkuehbach changed the title #243 adding correctors #243 Sprint22 Add stigmation correctors, Remove unnecessary EM-Base Classes, Open NXcomponent for other techniques Jun 27, 2024
atomprobe-tc and others added 4 commits June 27, 2024 18:38
…more meaty base class NXimage_set, the partner in crime for NXspectrum_set
…asses

Fused NXimage_r_set, NXimage_c_set, NXimage_r_set_diff into a single …
Promote component_em to generally usable base class NXcomponent
@mkuehbach mkuehbach changed the title #243 Sprint22 Add stigmation correctors, Remove unnecessary EM-Base Classes, Open NXcomponent for other techniques #243 Sprint22 Add stigmation correctors, removed unnecessary EM-Base Classes, opened NXcomponent for other techniques Jul 4, 2024
@mkuehbach
Copy link
Collaborator Author

This PR is ready to get merged into fairmat.

However, it introduces breaking changes in the data model of some em examples which requires feature updates and fixes to be made in pynxtools-em. Currently, there is vacation time, a basesection sprint is planned to happen right after the vacation time, which should then get all the focus. In summary, there is now a two week gap in which the changes for em can be implemented without introducing breaking changes on the main development line.

Therefore, I will close this PR and merge sprint22_corrector into sprint23_em_v3. This branch is a clone of today's fairmat branch such that it can be used to implement and test a specific pynxtools definition version instead of the mainline fairmat. This avoids breaking examples, enables to use the two week gap for an EM sprint.

An additional advantage is that also sprint22_microstructure can be merged into sprint23_em_v3 such that the entire package of em and microstructure description related changes can be used in the same pynxtools development. This is relevant because also feature additions of the MTex exporting is planned that require sprint22_microstructure.

@mkuehbach mkuehbach closed this Jul 30, 2024
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