Emapm cgms summer2025#395
Closed
mkuehbach wants to merge 37 commits into
Closed
Conversation
…eserved NeXus keyword versus renaming many of these to index_offset, indices_*
…se NX_POSINT, NX_UINT, harmonizing that all index and indices use NX_INT
…plus proofreading
…x is present writing tsl and mtex
…per decision of the NIAC nexusformat#1423 (comment)
…r thinking further about grouping NXevent_data instance or not I thought it could also be a valid argument that as why should the standard force people to not have thousands of groups ending up at the same level of the hierarchy. Clearly, there is nothing which prevents sb from doing this for the example of HDF5 but performance-wise this is problematic, there are multiple examples even in commercial microscope software solutions where indeed HDF5 files with thousands and more groups at the same level are stored using HDF5, while there is overhead involved in this and searching by humans might be ineffective it is still a valid HDF5 file according to the HDF5 data model. Also NeXus with allowing an (NXentry) de facto allows to make an instance where thousands of e.g. entry1, entry2, ... are stored in the same HDF5 file also here no attempt has been made to build another group to just suggest strongly that people avoid this practice. Therefore, I removed one layer of indentation within NXem_measurement so that in an instance one now can have instrument, event1, event2, .. It is a design issue with NeXus that when we accept and wish that one cannot use NXobject as a plain structuring element but at the same time does not wish ones content to become at the schema level already non verified like when using an NXcollection there is no practice to group content other than making a new base class such for the sake of it holding the grouping.
…oot of an app or class def
…Type: any with docstring Instances should by nameType partial and prefixID, lot 1, apm and cg
…sion with the NIAC when specifically as documented in https://github.com/nexusformat/definitions/pull/1415/files
…starting with the inline comments, next step, address remaining comments that were made directly in the PR discussion
…s_computer, ii) review remaining large EM base classes
…rsion of the EM pr
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
We failed to fetch the diff for pull request #395
You can try again by commenting this pull request with @sourcery-ai review, or contact us for help.
Collaborator
Author
|
Superseeded by #400 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings back changes from NIAC voting on: