Skip to content
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

Cast empty array to correct type before adding row #461

Merged
merged 16 commits into from
Nov 23, 2022

Conversation

lawrence-mbf
Copy link
Collaborator

Fixes #458

How to test the behavior?

See #458

Checklist

  • Have you ensured the PR description clearly describes the problem and solutions?
  • Have you checked to ensure that there aren't other open or previously closed Pull Requests for the same change?
  • If this PR fixes an issue, is the first line of the PR description fix #XX where XX is the issue number?

@lawrence-mbf lawrence-mbf self-assigned this Aug 17, 2022
@codecov
Copy link

codecov bot commented Aug 17, 2022

Codecov Report

Merging #461 (07d929e) into master (75e9dde) will decrease coverage by 0.30%.
The diff coverage is 89.17%.

@@            Coverage Diff             @@
##           master     #461      +/-   ##
==========================================
- Coverage   87.56%   87.25%   -0.31%     
==========================================
  Files         128      128              
  Lines        5348     5250      -98     
==========================================
- Hits         4683     4581     -102     
- Misses        665      669       +4     
Impacted Files Coverage Δ
+io/getRefData.m 77.77% <0.00%> (ø)
+io/parseAttributes.m 86.95% <0.00%> (ø)
+io/parseDataset.m 85.18% <0.00%> (ø)
+types/+untyped/+datapipe/BoundPipe.m 79.71% <0.00%> (+0.72%) ⬆️
+tests/+unit/tutorialTest.m 81.81% <40.00%> (-12.63%) ⬇️
+types/+util/+dynamictable/addVecInd.m 88.00% <50.00%> (-3.31%) ⬇️
+types/+untyped/+datapipe/BlueprintPipe.m 77.06% <62.50%> (+2.06%) ⬆️
+types/+util/correctType.m 90.47% <75.00%> (ø)
+types/+util/+dynamictable/checkConfig.m 92.79% <92.72%> (-0.21%) ⬇️
+file/mapType.m 94.28% <100.00%> (ø)
... and 15 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

When concatenating to an empty double array "[]", MATLAB usually "adopts" the type from the new element
instead of attempting to cast to the double type.

As it turns out, logical values are a rare type where concatenating a "false" or "true" to an empty
array actually returns the double value of the empty array type. This added check fixes this
specific case.
- Blueprint datapipes now support the same load() and append() calls one could use with Bound
datapipes. These simply operate with the data cached in memory.
- Added a few fixes and special cases that Dynamic Tables did not actually check for.
- Support tables when getting row
@lawrence-mbf lawrence-mbf force-pushed the 458-prevent-type-erasure-on-add-row branch from 01ab16f to fad8cb3 Compare November 14, 2022 15:26
vectorindex references where needed.
blueprint pipe vector dimensions are not necessarily aligned with its axis dimension.
This is intentional as the axis dimension reflects the data dimension when written to
disk (to fit with how it works with the bound pipe).
MatNWB -> NWB since errors thrown in MATLAB make this a redundant specifier as a domain.
@lawrence-mbf lawrence-mbf marked this pull request as ready for review November 14, 2022 19:47
@lawrence-mbf lawrence-mbf merged commit cf16d37 into master Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Dynamictable.addRow with logical type
2 participants