fail early for FixedSizeArray subtypes we can't handle
#99
Conversation
This file contains 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
Before this PR `collect_as` had to use a type assert on the return
value to make sure the return value is of the type requested by the
user. This is necessary because of UnionAll types with non-default
constraints on their type parameters, such as
`FixedSizeVector{T} where {T <: Number}`, because accessing the
specific constraint is not currently possible in Julia in a supported
way.
This change removes the type assert, instead throwing early, as soon as
it is recognized that the requested return type is not among the
allowed types, which are known-good, in the sense that the return value
we produce ends up being of the requested type as expected.
Benefits:
* a type assert of nonconcrete type is not required any more (those can
be expensive)
* the throw happens earlier than before
Future PRs will possibly apply the same mechanic to some constructors,
in addition to `collect_as`.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #99 +/- ##
==========================================
+ Coverage 95.96% 96.07% +0.11%
==========================================
Files 3 3
Lines 248 255 +7
==========================================
+ Hits 238 245 +7
Misses 10 10 ☔ View full report in Codecov by Sentry. |
giordano
approved these changes
Feb 11, 2025
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.
Before this PR
collect_ashad to use a type assert on the return value to make sure the return value is of the type requested by the user. This is necessary because of UnionAll types with non-default constraints on their type parameters, such asFixedSizeVector{T} where {T <: Number}, because accessing the specific constraint is not currently possible in Julia in a supported way.This change removes the type assert, instead throwing early, as soon as it is recognized that the requested return type is not among the allowed types, which are known-good, in the sense that the return value we produce ends up being of the requested type as expected.
Benefits:
Future PRs will possibly apply the same mechanic to some constructors, in addition to
collect_as.