Repository navigation
refactor: remove duplicate structured field helper - #183
Merged
sromoam merged 1 commit intoAug 6, 2026
Merged
Conversation
This was referenced Jul 31, 2026
Contributor
|
@hugosmoreira thank you for this PR! |
sromoam
marked this pull request as ready for review
August 6, 2026 18:51
sromoam
approved these changes
Aug 6, 2026
sromoam
left a comment
Contributor
There was a problem hiding this comment.
Looks good, looks like we found a legit dead code path that is safe for delete.
This was referenced Aug 6, 2026
Merged
sromoam
added a commit
to Pybsama/stickler
that referenced
this pull request
Aug 13, 2026
Closes awslabs#213 for everything currently mergeable. awslabs#159 and awslabs#183 have shipped with no entry; awslabs#146 and awslabs#198 are still open, so their entries belong with those PRs rather than here. awslabs#159 gets three entries, two Fixed and one Changed. Verified each against the merge parent (b78c1f0) rather than trusting the issue text, which turned out to describe the export shape wrongly: to_json_schema() before {"type": "string"} after {"type": ["string", "null"]} model_json_schema() before {"type": "string"} after {"anyOf": [{"type": "string"}, {"type": "null"}]} The issue said both emit `anyOf`. Only `model_json_schema()` does -- that is Pydantic rendering `Optional[T]` -- while `to_json_schema()` uses the list form. The entry documents both, since a reader feeding either into a validator needs the one they actually call. Also confirmed the other claims held before writing them up: explicit `null` now accepted for an optional schema-built field (ValidationError before), required fields still reject `None`, re-import still idempotent, and `is_structured_field_type()` now returns True for `Optional[SomeModel]` where it returned False. That last one needed a FieldInfo rather than a bare annotation to reproduce -- passing the annotation returns False on both sides, which would have read as the claim being wrong. awslabs#183 deliberately gets no entry: 44 lines of a duplicate private helper removed, no runtime change. The issue asked for a decision rather than a default, so the convention is now written down in RELEASING.md -- entries are for changes a user can observe, internal refactors are git log's job. 0.6.0's notes already followed this; it just had not been stated. Added a CHANGELOG checkbox to the PR template, phrased so "not needed" is an explicit choice rather than an omission. Every gap in awslabs#213 is a PR that passed review without one, which points at the process rather than the authors. Suite 1746 passed, 2 skipped; mkdocs clean; the template's relative link to RELEASING.md resolves.
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.
Issue #, if available:
Closes #163
Description of changes:
Removes the unused
FieldHelper.is_structured_field_typeimplementation and the imports that only supported it.ConfigurationHelper.is_structured_field_typeremains the single structured-field classifier used by the model pipeline. Unlike the removed duplicate, it also handlestyping.Unionand PEP 604 union annotations. This is a maintenance cleanup with no runtime behavior change because the removed method had no callers.Validation:
git diff --checkpassedBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.