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

Force new blocks to be contiguous #221

Merged
merged 3 commits into from
Oct 21, 2016
Merged

Force new blocks to be contiguous #221

merged 3 commits into from
Oct 21, 2016

Conversation

bernie-simon
Copy link
Contributor

The code in asdf assumes a block will be a contiguous block of memory. But recent versions of numpy have used views more heavily and now there are cases when an object instantiated as a bock is not contiguous, which leads to errors when the block is used. The modified code calls numpy.ascontiguousarray to make sure the block memory is contiguous.

This is a revised version of the branch, as several other irrelevant changes were put in the previous version of the branch.

The code in asdf assumes a block will be a contiguous block of
memory. But recent versions of numpy have used views more heavily and
now there are cases when an object instantiated as a bock is not
contiguous, which leads to errors when the block is used. The modified
code calls numpy.ascontiguousarray to make sure the block memory is
contiguous.

This is a revised version of the branch, as several other irrelevant
changes were put in the previous verion of the branch.
Testing makes it look like ascontiguousarray does not check this flag,
so now the code does the check before calling it. Not doing this check
can lead to having multiple copies of the same data which is a bad
thing and leads to file growth on copying.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 93.691% when pulling 0d669f5 on bernie-simon:contiguous_blocks into 5fc5720 on spacetelescope:master.

Rethinking the issues involved here made me change the if test to
check if the block being copied is an ndarray. If not, it can't be
non-contiguous, as that "feature" is only available for an ndarray.
@bernie-simon bernie-simon merged commit 1a2bf4a into asdf-format:master Oct 21, 2016
@bernie-simon
Copy link
Contributor Author

Closing out an old issue first raised by a problem encountered by Phil.

@bernie-simon bernie-simon deleted the contiguous_blocks branch October 21, 2016 13:49
@nden nden added this to the 1.1.1 milestone Oct 21, 2016
@nden nden added the bug label Oct 21, 2016
@nden
Copy link
Contributor

nden commented Oct 21, 2016

@bernie-simon I see the tests failed. Do you know how they failed?
Also please add a changelog entry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants