-
Notifications
You must be signed in to change notification settings - Fork 8
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
Base class inheritance example #66
base: fairmat
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @domna here my thoughts
@@ -0,0 +1,258 @@ | |||
doc: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For each docstring we should have at least the optional possibility but a field and keyword/command for this realized whereby we can add a reference to a concept of another ontology (OBO, BFO, EMMO) ...
Also the docstrings should really be inspected and cleaned from instructing to humans what they should format there, if we feel we need to express how things should be formatted behind a field we talk about constraint management
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I support this. We could add something like ontology_reference
or ontology_uri
as sub-field of doc
?
I like the idea of adding constraints (I'm thinking of describing formulas etc. properly here as well) but I have the feeling that currently its a bit out of our scope of our simple prototype example here.
Properties of an optical beam at a given location. | ||
category: base | ||
(NXbeam)NXbeam_optical: | ||
incident_polarization(NX_NUMBER): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would tend to move this to NXbeam_polarization and then add in this polarization base class the method whereby that polarization is described/parameterized e.g.
descriptor(NXCHAR):
enumeration: [stokes]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this to NXbeam_polarized
.
Could you elaborate on the described/parametrized notation you propose? I do not understand this.
chirp_GDD(NX_FLOAT): | ||
unit: NX_TIME | ||
doc: | | ||
Group delay dispersion of the pulse for linear chirp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is a linear chirp there is likely also a nonlinear chirp, would a base class NXsignal_processing_descriptor_chirp make sense (dont take that naive name of this base class for granted) but read "between the lines" conceptually?
Same story for frog?
I see one problem/inconvenience of a finer granularization maybe people need to navigate deeper but the benefit is stronger modularization, I am still not convinced though that I am overdoing it here with the balance between convenience and granularization
This is an example for
NXbeam
to discuss and implement an example for base class inheritance.I roughly divided the current NXbeam into multiple sub-classes. Feel free to suggest moving or adding fields. Also if you have an idea how to build a multiple inheritance example from this feel free to suggest something.
This is the current structure:
The
NXbeam_polarized_time_resolved
example doesn't really make sense. I just added it to show a possible formulation of multiple inheritance.@sherjeelshabih @Tommaso-Pincelli @tomio13 @mkuehbach @rettigl @sanbrock @RubelMozumder