Skip to content

Conversation

@bohJiang12
Copy link
Contributor

Update

This commit aims to fix issue #296 which expects following:

  1. Current MMIF 1.x uses long-form of annotation's id (e.g. instead of ann.id, rather view.id:ann.id across the whole MMIF world
  2. Promote the annotation's id which originally resides in AnnotationProperties class to the Annotation class and make it as a class property as View class did to its id.

Discussion

One issue was found while implementing, that is, the location of annotation's id in raw MMIF JSON file (the input/output source file for MMIF SDK) mismatches our expected location to it. The cause of such mismatch is that the annotation's id would naturally be put under AnnotationProperties class by default and uses the short-form id as is during deserialization. Therefore, it's not possible to achieve the first goal stated above.

Then, one feasible approach to target it is that we need to implement a "converter" utility as an interface between raw JSON MMIF and the SDK so that the raw JSON file can be converted to a desired format for the SDK. One advantage of the approach is that it requires minimal change for the SDK and it is isolated to the SDK.

@bohJiang12 bohJiang12 requested a review from keighrim October 3, 2024 16:38
@keighrim
Copy link
Member

I can't review this due to too much noisy changes from simple white space re-formatting. Please don't do code-restyling in a feature or fix patch branch...

@keighrim
Copy link
Member

force-pushed a rebased commit without whitespace changes.

@keighrim
Copy link
Member

This PR says (in the body) to fix #296.
That issue 296 addresses clamsproject/mmif#228 but mentions nothing about clamsproject/mmif#233 , however this PR aims to fix both at the same time. (I think I miss-spoke the goal for this PR when I assigned to @bohJiang12 )

Since the promotion of the id field will involve update in the MMIF jsonschema, it can't be done simply on the SDK-only level. Hence, I think this PR needs to focus on the "long-form" issue only for now.

@keighrim
Copy link
Member

closing without merge as the PR contains "out of scope" material.

@keighrim keighrim closed this May 26, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in infra May 26, 2025
@keighrim keighrim deleted the 296-update-id branch November 19, 2025 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants