feat: unify the fields for sparse and dense array #2578
Conversation
|
This PR closes: #2438 |
Done |
65fafbe to
c36eeef
Compare
|
@nan-wang Something new is happening. |
|
this change will affect some of our core code: |
Codecov Report
@@ Coverage Diff @@
## master #2578 +/- ##
==========================================
+ Coverage 86.58% 89.37% +2.79%
==========================================
Files 148 146 -2
Lines 8943 8969 +26
==========================================
+ Hits 7743 8016 +273
+ Misses 1200 953 -247
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
@nan-wang should be ready |
|
@bwanglzu Have you rerun the |
|
@nan-wang I ran the docker command as was specified in |
| @@ -253,7 +253,7 @@ def test_jina_document_to_pydantic_document_sparse(): | |||
| == pydantic_doc.embedding.sparse.values.buffer.decode() | |||
| ) | |||
| assert ( | |||
| jina_doc['embedding']['sparse']['values']['shape'] | |||
| list(map(int, jina_doc['embedding']['sparse']['values']['shape'])) | |||
There was a problem hiding this comment.
@JoanFM after type changed, the shape is str, while the shape passed to pandantic is int
There was a problem hiding this comment.
the shape in proto is a repeated int64
There was a problem hiding this comment.
As per proto3 JSON spec, uint64/int64 fields should be printed as decimal strings. See:
https://developers.google.com/protocol-buffers/docs/proto3#json
The reason is that uint64/int64 is not part of JSON spec and many JSON libraries only support double precision. To prevent precision loss our proto3 JSON spec requires serializers to put int64/uint64 values in strings.
checkout here
There was a problem hiding this comment.
another option is to use int32 to represent the shape, how do you think @nan-wang ?
Latency summaryCurrent PR yields:
Breakdown
Backed by latency-tracking. Further commits will update this comment. |
|
unified all |
resolved #2438