Skip to content

Conversation

@ajnelson-nist
Copy link
Member

@ajnelson-nist ajnelson-nist commented Nov 27, 2024

This PR does not take the last step of adding the --strict flag in type review.

This PR satisfies a run of the type checker in strict mode against the script /example.py and all of the library functions that script depends on. Additionally, all touched __init__ methods observed to not have *args & **kwargs catch-alls had those added and incorporated into super() calls (which will assist with tying to, e.g., knowledge base namespace helpers available in the ultimate parent class UcoThing).

The first patch also adjusted one class position, moving MessageThread under ObservableObject instead of UcoObject. The third patch does the same for CaseInvestigation, entailing a few more classes.

Two effects reflected back into the example script are:

  1. The EXIF dictionary incorporation logic needed its own keyword parameter, because the **kwargs catch-all parameter being used was an atypical usage versus the pass-through-to-superclass functionality of **kwargs.
  2. Compilation, ContextualCompilation, and their subclasses now must be initialized with at least one object UcoObject linked in the core_objects keyword parameter. Documentation is added to note this is to guarantee minimum cardinality constraints in the ontology are always satisfied.

@ajnelson-nist ajnelson-nist force-pushed the support_strict_review_on_example_py branch from 33f0844 to 2f47498 Compare November 27, 2024 19:48
This patch does not take the last step of adding the `--strict` flag in
type review.

This patch satisfies a run of the type checker in strict mode against
the script `/example.py` and all of the library functions that script
depends on.  Additionally, all touched `__init__` methods observed to
not have `*args` & `**kwargs` catch-alls had those added and
incorporated into `super()` calls (which will assist with tying to,
e.g., knowledge base namespace helpers available in the ultimate parent
class `UcoThing`).

One effect reflected back into the example script is that the EXIF
dictionary incorporation logic needed its own keyword parameter, because
the `**kwargs` catch-all parameter being used was an atypical usage
versus the pass-through-to-superclass functionality of `**kwargs`.

This patch also adjusted one class position, moving `MessageThread`
under `ObservableObject` instead of `UcoObject`.  A follow-on patch will
do the same for `CaseInvestigation`.

A follow-on patch will regenerate Make-managed files.

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
@ajnelson-nist ajnelson-nist force-pushed the support_strict_review_on_example_py branch from 2f47498 to 86de46e Compare November 27, 2024 19:49
Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
…Object

One effect reflected back into the example script is that `Compilation`,
`ContextualCompilation`, and their subclasses now must be initialized
with at least one object `UcoObject` linked in the `core_objects`
keyword parameter.  Documentation is added to note this is to guarantee
minimum cardinality constraints in the ontology are always satisfied.

A follow-on patch will regenerate Make-managed files.

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
@ajnelson-nist ajnelson-nist marked this pull request as ready for review November 27, 2024 20:20
@ajnelson-nist ajnelson-nist requested review from a team and fabrizio-turchi November 27, 2024 20:20
@fabrizio-turchi
Copy link
Contributor

Hi Alex,
I don't have any objections to approve this PR. To tell you the truth I don't well understand the reasons why you introduced the new classes Compilation , ContextualCompilation and EnclosingCompilation to guarantee minimum cardinality constraints in the ontology. It's a rather selfish consideration but the XML parsers seem to work as usual. Thanks!

@ajnelson-nist
Copy link
Member Author

@fabrizio-turchi The reason for introducing those classes into the Python class hierarchy is that they are in the UCO ontology class hierarchy, but hadn't been implemented in the Python class hierarchy yet. They are parent classes of case-investigation:Investigation, and seemed like more appropriate places to house some of the Python code around bundling objects.

I'm glad there was no downstream effect. That was one of the goals of this PR.

@kchason kchason merged commit 89649a3 into main Dec 2, 2024
4 checks passed
@kchason kchason deleted the support_strict_review_on_example_py branch December 2, 2024 14:35
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.

4 participants