test(pyamber): add unit tests for attribute_type_utils#6833
Open
Amer-Mukhtar wants to merge 1 commit into
Open
test(pyamber): add unit tests for attribute_type_utils#6833Amer-Mukhtar wants to merge 1 commit into
Amer-Mukhtar wants to merge 1 commit into
Conversation
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 434 | 0.265 | 22,816/28,699/28,699 us | 🟢 -17.9% / 🔴 +78.5% |
| 🔴 | bs=100 sw=10 sl=64 | 924 | 0.564 | 105,233/156,049/156,049 us | 🔴 +19.2% / 🔴 +45.3% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,101 | 0.672 | 914,839/958,871/958,871 us | ⚪ within ±5% / 🟢 -9.2% |
Baseline details
Latest main 0ba5d1a from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 434 tuples/sec | 418 tuples/sec | 760.2 tuples/sec | +3.8% | -42.9% |
| bs=10 sw=10 sl=64 | MB/s | 0.265 MB/s | 0.255 MB/s | 0.464 MB/s | +3.9% | -42.9% |
| bs=10 sw=10 sl=64 | p50 | 22,816 us | 20,344 us | 12,781 us | +12.2% | +78.5% |
| bs=10 sw=10 sl=64 | p95 | 28,699 us | 34,938 us | 16,118 us | -17.9% | +78.1% |
| bs=10 sw=10 sl=64 | p99 | 28,699 us | 34,938 us | 19,074 us | -17.9% | +50.5% |
| bs=100 sw=10 sl=64 | throughput | 924 tuples/sec | 963 tuples/sec | 988.68 tuples/sec | -4.0% | -6.5% |
| bs=100 sw=10 sl=64 | MB/s | 0.564 MB/s | 0.588 MB/s | 0.603 MB/s | -4.1% | -6.5% |
| bs=100 sw=10 sl=64 | p50 | 105,233 us | 102,314 us | 101,448 us | +2.9% | +3.7% |
| bs=100 sw=10 sl=64 | p95 | 156,049 us | 130,861 us | 107,410 us | +19.2% | +45.3% |
| bs=100 sw=10 sl=64 | p99 | 156,049 us | 130,861 us | 120,258 us | +19.2% | +29.8% |
| bs=1000 sw=10 sl=64 | throughput | 1,101 tuples/sec | 1,082 tuples/sec | 1,023 tuples/sec | +1.8% | +7.6% |
| bs=1000 sw=10 sl=64 | MB/s | 0.672 MB/s | 0.66 MB/s | 0.624 MB/s | +1.8% | +7.6% |
| bs=1000 sw=10 sl=64 | p50 | 914,839 us | 926,537 us | 983,888 us | -1.3% | -7.0% |
| bs=1000 sw=10 sl=64 | p95 | 958,871 us | 978,638 us | 1,030,100 us | -2.0% | -6.9% |
| bs=1000 sw=10 sl=64 | p99 | 958,871 us | 978,638 us | 1,056,513 us | -2.0% | -9.2% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,460.91,200,128000,434,0.265,22816.13,28698.68,28698.68
1,100,10,64,20,2163.64,2000,1280000,924,0.564,105232.52,156049.35,156049.35
2,1000,10,64,20,18168.18,20000,12800000,1101,0.672,914839.37,958870.50,958870.50
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6833 +/- ##
=========================================
Coverage 76.28% 76.28%
Complexity 3452 3452
=========================================
Files 1161 1161
Lines 45916 45916
Branches 5099 5099
=========================================
Hits 35025 35025
Misses 9327 9327
Partials 1564 1564
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds missing unit-test coverage for the Python worker’s core.models.schema.attribute_type_utils conversion helpers, ensuring Arrow ↔ AttributeType mapping (including LARGE_BINARY’s metadata-based encoding) is validated and protected against regressions.
Changes:
- Introduces a new pytest module covering
detect_attribute_type_from_arrow_fieldacross supported Arrow types, timestamp units, metadata precedence, and error behavior for unsupported types. - Adds coverage for
create_arrow_field_with_metadata, includingLARGE_BINARYmarker metadata behavior and round-trip preservation through Arrow schemas.
Comments suppressed due to low confidence (1)
amber/src/test/python/core/models/schema/test_attribute_type_utils.py:179
- Similar to the earlier metadata check: comparing
field.metadatato a plain dict can be fragile. Prefer asserting the presence/value of thetexera_typemarker to avoid version-specific equality behavior.
def test_unicode_name_survives_on_a_large_binary_field(self):
field = create_arrow_field_with_metadata("列名", AttributeType.LARGE_BINARY)
assert field.name == "列名"
assert field.metadata == LARGE_BINARY_METADATA
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+148
to
+151
| def test_large_binary_field_carries_marker_metadata(self): | ||
| field = create_arrow_field_with_metadata("payload", AttributeType.LARGE_BINARY) | ||
| assert field.metadata == LARGE_BINARY_METADATA | ||
|
|
Comment on lines
+189
to
+193
| def test_non_attribute_type_raises_key_error(self, invalid_type): | ||
| """Anything that isn't an AttributeType member fails the mapping lookup.""" | ||
| with pytest.raises(KeyError): | ||
| create_arrow_field_with_metadata("f", invalid_type) | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
What changes were proposed in this PR?
amber/src/test/python/core/models/schema/test_attribute_type_utils.py, a new pytest spec for the Python worker'sattribute_type_utils, which previously had no dedicated unit tests.detect_attribute_type_from_arrow_field: every supported Arrow type, timestamps across all four units, thetexera_type: LARGE_BINARYmetadata marker taking precedence over the Arrow type, metadata that doesn't mark large binary falling back to the type mapping, and unmapped Arrow types raisingKeyError.create_arrow_field_with_metadata: everyAttributeType, metadata attached only forLARGE_BINARY, attribute names preserved verbatim (empty, unicode, emoji, long), and non-AttributeTypeinput raisingKeyError.LARGE_BINARYfield degrades it toSTRING.Any related issues, documentation, discussions?
Closes: #6635
How was this PR tested?
cd amber && pytest src/test/python/core/models/schema/test_attribute_type_utils.py -v, expect all 67 tests passing.Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude