Skip to content
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

Fix mmif.serialize pythonic behavior #304

Open
wants to merge 3 commits into
base: 2.x
Choose a base branch
from
Open

Conversation

bohJiang12
Copy link
Contributor

@bohJiang12 bohJiang12 commented Aug 22, 2024

Description

This PR aims to fix #295 which pointed out that dict-like (and list-like) classes in mmif.serialize package need to hold python's dict (and list) properties.

Overview of classes in mmif.serialize

  • MmifObject: a base class for MMIF objects that are ''dict-like''
  • DataList: a base class for MMIF data fields that are ''list-like''
  • DataDict: a base class for MMIF data fields that are ''dict-like''

Then, the following classes are defined and categorized into either ''dict-like''
or ''list-like'' child classes:

dict-like:
- Mmif: a class for MMIF objects
- MmifMetaData: a class for MMIF metadata
- View: a class for MMIF view
- ViewMetaData: a class for MMIF view metadata
- ErrorDict: a class for a specific view that contains error
- ContainsDict: a class for View's 'contains' field
- Annotation & Document: a class for MMIF annotation and document
- AnnotationProperties & DocumentProperties: a class for MMIF annotation properties
- Text: a class for Document's text field

list-like:
- DocumentsList: a class for a list of Document objects
- ViewsList: a class for a list of View objects
- AnnotationsList: a class for a list of Annotation objects

By doing so, any field of a ''dict-like'' class should be accessed if and
only if by either bracket [key] or .get(key). For a ''list-like'' class, the
elements are ordered and accessed by index, for example, [idx].

Note

As I discussed with @keighrim, we intentionally leave a "backdoor" method get_item for the abstract class DataList in order for achieving O(1) access to data (for example, a View or an Annotation) by its long_id.

What's new

  • Any subclass of MmifObject (i.e. dict-like class) has only two ways to access data via keys:

    • dict_like_obj[key]
    • dict_like_obj.get(key)
  • Any subclass of DataList (i.e. list-like) has below changes:

    • Access a data by index: list_like_obj[index]
    • However, making changes to existing data in the list is disabled now: list_like_obj[index] = new_value raises TypeError
    • The method that access a data by its key has been renamed from .get to .get_item

@bohJiang12 bohJiang12 changed the title 295-fix-pythonic-behavior Fix mmif.serialize pythonic behavior Aug 22, 2024
Copy link

codecov bot commented Aug 23, 2024

Codecov Report

Attention: Patch coverage is 82.35294% with 12 lines in your changes missing coverage. Please review.

Project coverage is 91.77%. Comparing base (2e17432) to head (0a4ce6c).
Report is 8 commits behind head on develop.

Files Patch % Lines
mmif/serialize/mmif.py 80.95% 4 Missing ⚠️
mmif/serialize/view.py 66.66% 4 Missing ⚠️
mmif/serialize/annotation.py 77.77% 2 Missing ⚠️
mmif/serialize/model.py 90.90% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #304      +/-   ##
===========================================
+ Coverage    90.74%   91.77%   +1.02%     
===========================================
  Files           10       10              
  Lines         1383     1398      +15     
===========================================
+ Hits          1255     1283      +28     
+ Misses         128      115      -13     
Flag Coverage Δ
unittests 91.77% <82.35%> (+1.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@keighrim keighrim changed the base branch from develop to 2.x August 25, 2024 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

make .get() and indexing ([]) more pythonic
1 participant