-
Notifications
You must be signed in to change notification settings - Fork 34
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
Mat index bug #249
base: main
Are you sure you want to change the base?
Mat index bug #249
Conversation
Fixes a couple of the formerly-broken tests!
See discussion in this issue: #248 |
@jonniedie polite ping re: this PR. |
test/runtests.jl
Outdated
@@ -132,7 +137,7 @@ end | |||
for T in [Int64, Int32, Float64, Float32, ComplexF64, ComplexF32] | |||
@test ComponentArray(a = T[]) == ComponentVector{T}(a = T[]) | |||
@test ComponentArray(a = T[], b = T[]) == ComponentVector{T}(a = T[], b = T[]) | |||
@test ComponentArray(a = T[], b = (;)) == ComponentVector{T}(a = T[], b = T[]) | |||
@test_broken ComponentArray(a = T[], b = (;)) == ComponentVector{T}(a = T[], b = T[]) |
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.
It's important we don't break this one because the behavior is relied upon in some some large simulation projects. This allows ComponentArray
s that match a nested model structure to be initialized when one of the internal models doesn't have integrable state.
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.
Gotcha, thanks. I'll take a look at it again when I get a chance.
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.
Fixed!
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.
@jonniedie Another polite ping. :-)
@jonniedie Another polite ping re: this PR. :-) It's ready to go from my perspective. |
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #249 +/- ##
==========================================
+ Coverage 73.88% 74.07% +0.18%
==========================================
Files 23 25 +2
Lines 743 787 +44
==========================================
+ Hits 549 583 +34
- Misses 194 204 +10 ☔ View full report in Codecov by Sentry. |
@jonniedie Another polite ping re: this PR. :-) |
No description provided.