WIP: generator accepts NdArray attribute #575

zhanyuanucb · 2021-12-20T20:59:19Z

This PR fixes #567.

Description of changes

In ontology_code_const.py:
- Add “NdArray” to COMPOSITE
- Add “shape” and “dtype” to SchemaKeyWords
In ontology_code_generator.py:
- modify parse_property to handle att_type == “NdArray”
- define parse_ndarray to parse ndarray attributes:
  - ndarray_value_type
- return a NdArrayProperty
In code_generation_objects.py
- add NdArrayProperty
In forte.data.ontology.core.py
- add FNdArray that wraps a numpy ndarray and store related properties like dtype and shape
validation_schema.json
- Add schema for NdArray attribute

Possible influences of this PR.

Describe what are the possible side-effects of the code change.

Test Conducted

tests/forte/data/ontology/ontology_code_generator_test.py
- test generated code for sample Entry containing NdArray attribute
tests/forte/data/ontology/ndarray_attribute_test.py:
- Test serialization
- Test setting array property

codecov · 2021-12-20T21:11:43Z

Codecov Report

Merging #575 (7599c13) into master (ab5248e) will increase coverage by 0.22%.
The diff coverage is 96.72%.

@@            Coverage Diff             @@
##           master     #575      +/-   ##
==========================================
+ Coverage   79.86%   80.09%   +0.22%     
==========================================
  Files         229      231       +2     
  Lines       16239    16452     +213     
==========================================
+ Hits        12970    13177     +207     
- Misses       3269     3275       +6

Impacted Files	Coverage Δ
forte/data/ontology/code_generation_objects.py	`92.67% <84.21%> (-0.39%)`	⬇️
...ests/forte/data/ontology/ndarray_attribute_test.py	`94.66% <94.66%> (ø)`
forte/data/ontology/core.py	`84.06% <100.00%> (+1.83%)`	⬆️
forte/data/ontology/ontology_code_const.py	`100.00% <100.00%> (ø)`
forte/data/ontology/ontology_code_generator.py	`90.09% <100.00%> (+0.37%)`	⬆️
...orte/data/ontology/ontology_code_generator_test.py	`100.00% <100.00%> (ø)`
...data/ontology/test_outputs/ft/onto/test_ndarray.py	`100.00% <100.00%> (ø)`
forte/data/base_pack.py	`77.77% <0.00%> (+0.42%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab5248e...7599c13. Read the comment docs.

forte/data/ontology/ontology_code_generator.py

forte/data/ontology/code_generation_objects.py

forte/ontology_specs/metric.json

hunterhector

The PR looks very good, only a few small comments.

hunterhector · 2022-01-07T18:24:28Z

forte/data/ontology/code_generation_objects.py

@@ -382,6 +383,45 @@ def to_field_value(self):
        return self.name


+class NdArrayProperty(Property):


I think we need some docstring for the class. I understand that many other classes in this file don't have that, but that's a historical mistake, let's add docstring as much as possible for future code.

forte/data/ontology/core.py

forte/data/ontology/ontology_code_generator.py

hunterhector · 2022-01-09T21:25:36Z

forte/data/ontology/validation_schema.json

@@ -95,6 +95,14 @@
                "value_type": {
                  "description": "Item type for the case of Dice attributes",
                  "type": "string"
+                },
+                "ndarray_dtype": {


do we have some tests to validate a JSON spec with this schema, hopefully, both success and failure cases?

Added a test against invalid shape.
Do you think we can rely on numpy to validate dtype?

it'd be great if schema can check the possible dtype values. Since this is done without actually running the code, while numpy validation happens late at run time.

sounds good.
I've added more tests against dtype and shape

the validation looks awesome, and thanks for the tests

forte/data/ontology/core.py

tests/forte/data/ontology/test_specs/test_invalid_ndarray_shape.json

* 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 <zhanyuan.zhang@petuum.com>

* Add AudioAnnotation entry to ontology system * WIP: generator accepts NdArray attribute (#575) * 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 <zhanyuan.zhang@petuum.com> * Resolve comments * Fix confusing types of range_annotation * Add test case for AudioAnnotation Co-authored-by: Suqi Sun <suqi.sun@petuum.com> Co-authored-by: Zhanyuan Zhang <32000378+zhanyuanucb@users.noreply.github.com> Co-authored-by: Zhanyuan Zhang <zhanyuan.zhang@petuum.com>

Zhanyuan Zhang and others added 21 commits November 1, 2021 18:42

added static methods to infer pack type

a7907cf

added unit tests for datapack type inference.

def940c

Merge branch 'master' of github.com:asyml/forte

b6e363a

Merge branch 'master' of github.com:asyml/forte

cc3423b

fixed issue#558

a6fa453

Merge branch 'master' of github.com:asyml/forte

4cb691c

Merge branch 'master' into issue#558

16bd32f

Merge branch 'master' of github.com:asyml/forte into issue#558

16a934b

fixed black: forte/data/selector.py

dd85896

Merge branch 'issue#558' of github.com:zhanyuanucb/forte into issue#558

4731eb3

fixed import-outside-toplevel

148a03b

Merge branch 'issue#558'

aca7510

Merge branch 'master' of github.com:asyml/forte

5d95063

WIP: waiting for ndarray supported

0cb0987

Ndarray -> ndarray

45b4ce0

values -> value

0193898

added ndarray to SUPPORTED_PRIMITIVES

d082966

Merge branch 'master' of github.com:asyml/forte into issue#567

a4d8069

WIP: generator accepts NdArray attribute

2ba663f

remove breakpoints

6e58a57

fixed None -> "None"

0449b44

hunterhector reviewed Dec 20, 2021

View reviewed changes

Zhanyuan Zhang added 7 commits December 21, 2021 01:18

WIP: designing NdArrayProperty

92d9eac

WIP: ndarray_size -> ndarray_shape

54c4355

fixed lint

e4afbba

fixed lint

bcf7e3b

added ndarray property test cases

5702eb1

removed irrelevant onto spec

cc86f95

fixed lint

3ab5c8f

Zhanyuan Zhang added 4 commits December 21, 2021 02:19

fixed black

5886cb5

fixed description

f0ec23f

added FNdArray, a wrapper class for NdArray metric

605e374

handle None dtype

672c54a

hunterhector marked this pull request as draft December 22, 2021 01:20

Zhanyuan Zhang added 5 commits December 22, 2021 03:12

removed Optional typing

d98ee2d

fixed description

c4f1279

added unit tests for ndarray attribute

f355196

fixed black

b8f7784

Merge branch 'master' of github.com:asyml/forte into issue#567

d5c4184

zhanyuanucb marked this pull request as ready for review January 5, 2022 18:33

Merge branch 'master' into issue#567

4229358

hunterhector reviewed Jan 9, 2022

View reviewed changes

Zhanyuan Zhang added 3 commits January 10, 2022 06:01

added doc string and more tests

e0f1a37

Merge branch 'issue#567' of github.com:zhanyuanucb/forte into issue#567

b729235

fixed type

33ebd58

hunterhector reviewed Jan 10, 2022

View reviewed changes

forte/data/ontology/core.py Outdated Show resolved Hide resolved

hunterhector reviewed Jan 10, 2022

View reviewed changes

tests/forte/data/ontology/test_specs/test_invalid_ndarray_shape.json Outdated Show resolved Hide resolved

Zhanyuan Zhang and others added 3 commits January 10, 2022 18:26

added reference to np.ndarray

c02d5f8

added unit tests for ndarray attribute against dtype, shape, and warning

a7e73a9

Merge branch 'master' into issue#567

7599c13

hunterhector approved these changes Jan 13, 2022

View reviewed changes

hunterhector merged commit 45b9551 into asyml:master Jan 13, 2022

zhanyuanucb mentioned this pull request Jan 13, 2022

Doc for ndarray attribute #588

Closed

zhanyuanucb deleted the issue#567 branch January 13, 2022 19:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: generator accepts NdArray attribute #575

WIP: generator accepts NdArray attribute #575

zhanyuanucb commented Dec 20, 2021 •

edited

Loading

codecov bot commented Dec 20, 2021 •

edited

Loading

hunterhector left a comment

hunterhector Jan 7, 2022

zhanyuanucb Jan 10, 2022

hunterhector Jan 9, 2022

zhanyuanucb Jan 10, 2022

hunterhector Jan 10, 2022

zhanyuanucb Jan 11, 2022

hunterhector Jan 13, 2022

		@@ -382,6 +383,45 @@ def to_field_value(self):
		return self.name


		class NdArrayProperty(Property):

WIP: generator accepts NdArray attribute #575

WIP: generator accepts NdArray attribute #575

Conversation

zhanyuanucb commented Dec 20, 2021 • edited Loading

Description of changes

Possible influences of this PR.

Test Conducted

codecov bot commented Dec 20, 2021 • edited Loading

Codecov Report

hunterhector left a comment

Choose a reason for hiding this comment

hunterhector Jan 7, 2022

Choose a reason for hiding this comment

zhanyuanucb Jan 10, 2022

Choose a reason for hiding this comment

hunterhector Jan 9, 2022

Choose a reason for hiding this comment

zhanyuanucb Jan 10, 2022

Choose a reason for hiding this comment

hunterhector Jan 10, 2022

Choose a reason for hiding this comment

zhanyuanucb Jan 11, 2022

Choose a reason for hiding this comment

hunterhector Jan 13, 2022

Choose a reason for hiding this comment

zhanyuanucb commented Dec 20, 2021 •

edited

Loading

codecov bot commented Dec 20, 2021 •

edited

Loading