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

Fix shape in View::deepCopyView #1389

Merged
merged 7 commits into from
Aug 12, 2024
Merged

Conversation

gunney1
Copy link
Contributor

@gunney1 gunney1 commented Jul 24, 2024

Deep-copying a view fails to set the destination shape to the source's.
This bug fix follows the data allocation with a call to correctly set the destination shape.
It also fixes comments and prefixes some variable names with "src" or "dst" to clarify their role in the copy.

@gunney1 gunney1 added bug Something isn't working Sidre Issues related to Axom's 'sidre' component labels Jul 24, 2024
@gunney1 gunney1 self-assigned this Jul 24, 2024
Copy link
Member

@kennyweiss kennyweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @gunney1

Would it be possible to add a regression test for this? e.g. a case that failed w/ the old way that passes w/ this fix.

@gunney1 gunney1 force-pushed the bugfix/gunney/deep-copy-view branch from 6240fe6 to ed269dd Compare August 9, 2024 04:29
@gunney1
Copy link
Contributor Author

gunney1 commented Aug 9, 2024

Thanks for the fix @gunney1

Would it be possible to add a regression test for this? e.g. a case that failed w/ the old way that passes w/ this fix.

Regression test added.

@gunney1 gunney1 force-pushed the bugfix/gunney/deep-copy-view branch from 7308381 to d1e34f5 Compare August 12, 2024 05:29
@gunney1 gunney1 merged commit c284875 into develop Aug 12, 2024
13 checks passed
@gunney1 gunney1 deleted the bugfix/gunney/deep-copy-view branch August 12, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Sidre Issues related to Axom's 'sidre' component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants