-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add ontologies for audio data #592
Conversation
* added static methods to infer pack type * added unit tests for datapack type inference. * fixed issue#558 * fixed black: forte/data/selector.py * fixed import-outside-toplevel * WIP: waiting for ndarray supported * Ndarray -> ndarray * values -> value * added ndarray to SUPPORTED_PRIMITIVES * WIP: generator accepts NdArray attribute * remove breakpoints * fixed None -> "None" * WIP: designing NdArrayProperty * WIP: ndarray_size -> ndarray_shape * fixed lint * fixed lint * added ndarray property test cases * removed irrelevant onto spec * fixed lint * fixed lint * fixed black * fixed description * added FNdArray, a wrapper class for NdArray metric * handle None dtype * removed Optional typing * fixed description * added unit tests for ndarray attribute * fixed black * added doc string and more tests * fixed type * added reference to np.ndarray * added unit tests for ndarray attribute against dtype, shape, and warning Co-authored-by: Zhanyuan Zhang <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #592 +/- ##
==========================================
+ Coverage 80.07% 80.32% +0.24%
==========================================
Files 234 235 +1
Lines 16538 16748 +210
==========================================
+ Hits 13243 13452 +209
- Misses 3295 3296 +1
Continue to review full report at Codecov.
|
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.
There are a few things we could follow up with this:
- Add a few test cases related to the newly updated condition branches, notice that test cases should cover most of these branches since bugs often happen in these less used branches. Check the codecov report if needed. Examples of not tested areas include the warning message, deleting
AudioAnnotation
, and other similar new conditional branches.
The other two items should have their own issues and tickets, could you create two issues based on them?
- We have generally covered the
get
function but haven't covered theget_data
function. The differences between these two functions:get
obtain the classes whileget_data
return them as primitive types and nested dict.get_data
also can handle m more complex requests. It is worth creating an issue regarding this. Basically, we should have theget_data
function also work but not simply raising NotImplementedError as Line935 in data_pack.py. - The
index
andbuild_coverage_index
functions should be polished to include AudioAnnotation as well. Related line is here:Line 1101 in 7adecba
def build_coverage_for(
require_annotations(AudioAnnotation) | ||
and isinstance(range_annotation, Annotation) | ||
): | ||
logger.warning( |
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.
Is this conditional branch (i.e. the warning branch) tested?
This PR fixes #591 .
Description of changes
New top level entry: AudioAnnotation
text
property withaudio
.New ontologies in base_ontology: Recording, AudioAnnotation
Test Conducted
A unit test
audio_annotation_test
is added to test all the new ontologies added.