-
Notifications
You must be signed in to change notification settings - Fork 778
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
feat(grpc): IO descriptors #2884
Conversation
Hello @aarnphm, Thanks for updating this PR. There are currently no PEP 8 issues detected in this PR. Cheers! 🍻 Comment last updated at 2022-08-14 00:01:59 UTC |
50f21e6
to
997d5fd
Compare
Codecov Report
@@ Coverage Diff @@
## grpc #2884 +/- ##
==========================================
+ Coverage 59.77% 60.22% +0.45%
==========================================
Files 123 105 -18
Lines 9839 9464 -375
Branches 0 1656 +1656
==========================================
- Hits 5881 5700 -181
+ Misses 3958 3439 -519
- Partials 0 325 +325
|
a16c8d8
to
cd3c0d3
Compare
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
cd3c0d3
to
051429e
Compare
migrate setup.cfg to pyproject.toml Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
90f806e
to
de74ee7
Compare
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
# https://numpy.org/doc/stable/user/basics.byteswapping.html#introduction-to-byte-ordering-and-ndarrays | ||
bytesorder = "C" # default from numpy (C-order) | ||
|
||
self._bytesorder: t.Literal["C", "F", "A"] = bytesorder |
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.
remove _bytesorder?
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.
_bytesorder can be used for np.frombuffer.
if shapepb: | ||
shape = tuple(shapepb) | ||
if self._shape: | ||
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.
When force_shape is True, we should always use the shape specified when creating the Numpy io descriptor, not using the shape in request. Let's just raise an exception if the specified shape is different from the shape in the request if enforce shape is True. Make sure to handle the -1
case.
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 is checked on L410 right?
f"'enforce_dtype={self._enforce_dtype}', using '{self._dtype}'..." | ||
) | ||
dtype = self._dtype | ||
field_values = npdtype_to_fieldpb_map()[dtype] |
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 is not the right behavior. let's just convert the numpy array from proto first, and then check with np.can_cast. reuse the validation function above.
dtype=dtype, | ||
count=num_entries, | ||
offset=0, | ||
).reshape(shape) |
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.
add self.validate_array here
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 need to right? we already cast the dtype and reshape it with shape
# validate gRPC content type if content type is specified | ||
validate_content_type(context, self) | ||
|
||
field = ast.literal_eval(get_field(request, self)) |
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.
why do we need this?
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.
You can an put a bytes string.
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.
We should also allow a bytes field in all of the message.
See bentoml#2771 bentoml#2884 add gRPC context and types add descriptor metaclass add metaclass field for proto value verify_ndarray takes shape and dtype update types imports to public API Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 bentoml#2906 Co-authored-by: Sadab Hafiz <sadabhafizny@gmail.com> Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com> Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 bentoml#2906 Co-authored-by: Sadab Hafiz <sadabhafizny@gmail.com> Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com> Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 bentoml#2906 Co-authored-by: Sadab Hafiz <sadabhafizny@gmail.com> Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com> Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 bentoml#2906 Co-authored-by: Sadab Hafiz <sadabhafizny@gmail.com> Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com> Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 bentoml#2906 Co-authored-by: Sadab Hafiz <sadabhafizny@gmail.com> Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com> Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 bentoml#2906 Co-authored-by: Sadab Hafiz <sadabhafizny@gmail.com> Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com> Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
See bentoml#2771 bentoml#2884 bentoml#2906 Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
adding gRPC support for the remaining of the IO descriptor
Pandas will be handled by @Proto007
Test will be with a follow up PR.
This PR also remove
setup.cfg
and usepyproject.toml
. We will also lock version of setuptools==63.4.0 until 64+ is stable.since
bentoml/_version.py
is always generated and included inside the distribution, we should use that for__version__
instead of usingimportlib.metadata
. This will also avoidPackageNotFoundError
if bentoml is not yet installed, but attributes for version were accessed before.