Mpes readd base classes - #53
Conversation
mkuehbach
left a comment
mkuehbach
left a comment
There was a problem hiding this comment.
Left a few comments to support why some of these base class changes were removed before the code camp, now the situation is tricky: Most what is here proposed I think is ready to go if not all but then we should strongly revise the changes later albeit that could have happened in between the code camp and until now, in the interest of the vacation keep the changes minimal but lets write down the commit here to be source to refactor it with fresh energy before the nexus code and especially with hopefully then area A feedback.
|
The branch for this pull request was neither rebased against nor merged with 3b8f46a on fairmat. |
b95a151 to
9db7a87
Compare
Sorry, forgot this. It is rebased and still working with the example. |
I agree. I don't really have time to go through your comments. But let's keep them as a reference and have a more detailed discussion in #52 (I also added a comment there, that this here should be merged there, revisited and discussed) |
… version of yaml. Removing unintensional comments
|
Since we have substantial changes on the NXsample base class now, we should exclude any changes to NXsample from this PR. Next step is to refactor NXmpes to account for the new sample base class. Aside from that, LGTM. |
Regeneration of the nexus file for fixing the changes coming from old…
Since we have substantial changes on the NXsample base class now, here we remove any changes to NXsample in this PR.
|
This discussion will be continued in #52 |
sanbrock
left a comment
sanbrock
left a comment
There was a problem hiding this comment.
As discussed, although the removal of these properties were accidental and need to merge now back, they shall be refactored and remodelled.
review shall be continued in PR#52 where the refractoring will continue
6 participants
This readds the base classes removed in 843283a and 4b064d9
@mkuehbach @sanbrock Initially it did not work but I just checked the old file versions (from https://github.com/FAIRmat-NFDI/nexus_definitions/tree/ff35ff729aed3054e59c52e487fce3f54a30f1bb, which is the current reference in pynxtools==0.0.5) for NXaperture, NXdetector, NXmpes and, NXcollectioncolumn. This now passes the mpes examples without warning. Could be still that there are fields which we just don't use in the example. Could you check if I don't remove anything essential which was coming from NIAC? Other than that it should be fine from my side