-
Notifications
You must be signed in to change notification settings - Fork 60
Add a new optional argument tid in DataStore.add_entries #779
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #779 +/- ##
==========================================
+ Coverage 81.02% 81.09% +0.06%
==========================================
Files 249 249
Lines 18716 18748 +32
==========================================
+ Hits 15164 15203 +39
+ Misses 3552 3545 -7
Continue to review full report at Codecov.
|
forte/data/data_store.py
Outdated
entry: raw entry data in the list format. | ||
|
||
Raises: | ||
ValueError: raised when the entry type's sorting function is not |
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.
No ValueError
is raised in this function, I only see KeyError
…nk in _add_entry_raw()
num_link = len( | ||
self.data_store._DataStore__elements["forte.data.ontology.top.Link"] | ||
) | ||
self.assertEqual(num_link, 2) |
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.
should probably test whether the tid
is assigned correctly?
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.
added the test.
forte/data/data_store.py
Outdated
@@ -384,12 +396,13 @@ def _new_group(self, type_name: str, member_type: str) -> List: | |||
Args: | |||
type_name: The fully qualified type name of the new entry. | |||
member_type: Fully qualified name of its members. | |||
tid: ``tid`` of the ``Group`` entry. |
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.
This, and other occurrences of tid
, should expalin that tid
can be optional and will be auto-assigned.
This PR fixes #775 and #781
Description of changes
add_link_raw
andadd_group_raw
_add_entry_raw
to help modularizingadd_annotation_raw
,add_link_raw
andadd_group_raw
Possible influences of this PR.
Future functions that add new types of entries in raw format can use
_add_entry_raw
Test Conducted
add_link_raw
andadd_group_raw