Skip to content

Commit

Permalink
change back to defaulting to memmap, but raise warning on passing cop…
Browse files Browse the repository at this point in the history
…y_arrays
  • Loading branch information
zacharyburnett committed Jul 15, 2024
1 parent 149f1ed commit 231197c
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 6 deletions.
17 changes: 16 additions & 1 deletion asdf/_asdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ def __init__(
ignore_version_mismatch=True,
ignore_unrecognized_tag=False,
ignore_implicit_conversion=NotSet,
memmap=False,
memmap=NotSet,
copy_arrays=NotSet,
lazy_load=True,
custom_schema=None,
):
Expand Down Expand Up @@ -117,6 +118,13 @@ def __init__(
memmap : bool, optional
When `True`, when reading files, attempt to memmap underlying data
arrays when possible. When set, this argument will override
``copy_arrays``. The default will change to ``False`` in an upcoming
ASDF version.
copy_arrays : bool, optional
Deprecated; use ``memmap`` instead.
When `False`, when reading files, attempt to memmap underlying data
arrays when possible.
lazy_load : bool, optional
Expand Down Expand Up @@ -170,6 +178,13 @@ def __init__(
self._fd = None
self._closed = False
self._external_asdf_by_uri = {}
# if memmap is set, it overrides copy_arrays
if copy_arrays is not NotSet:
warnings.warn("copy_arrays is deprecated; use memmap instead", AsdfDeprecationWarning)
if memmap is NotSet:
memmap = not copy_arrays
elif memmap is NotSet:
memmap = True
self._blocks = BlockManager(uri=uri, lazy_load=lazy_load, memmap=memmap)
# this message is passed into find_references to only warn if
# a reference was found
Expand Down
10 changes: 9 additions & 1 deletion asdf/_tests/test_array_blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -811,16 +811,24 @@ def filename_with_array(tmp_path_factory):
@pytest.mark.parametrize(
"open_kwargs,should_memmap",
[
({}, False),
({}, True),
({"memmap": True}, True),
({"memmap": False}, False),
({"copy_arrays": True}, False),
({"copy_arrays": False}, True),
({"memmap": True, "copy_arrays": True}, True),
({"memmap": True, "copy_arrays": False}, True),
({"memmap": False, "copy_arrays": True}, False),
({"memmap": False, "copy_arrays": False}, False),
],
)
def test_open_no_memmap(filename_with_array, open_kwargs, should_memmap):
"""
Test that asdf.open does not (or does) return memmaps for arrays
depending on a number of arguments including:
default (no kwargs)
copy_arrays
memmap (overwrites copy_arrays)
memmap
"""
with asdf.open(filename_with_array, lazy_load=False, **open_kwargs) as af:
Expand Down
12 changes: 8 additions & 4 deletions docs/asdf/arrays.rst
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,10 @@ different compression algorithm when writing the file out again.
Memory mapping
--------------

Internal array data can be memory mapped using `numpy.memmap`, by setting
``memmap=True`` in either the `AsdfFile` constructor or `asdf.open`. This allows
for the efficient use of memory even when reading files with very large arrays.
The use of memory mapping means that the following usage pattern is not permitted:
By default, all internal array data is memory mapped using `numpy.memmap`. This
allows for the efficient use of memory even when reading files with very large
arrays. The use of memory mapping means that the following usage pattern is not
permitted:

.. code::
Expand All @@ -290,3 +290,7 @@ The use of memory mapping means that the following usage pattern is not permitte
Specifically, if an ASDF file has been opened using a ``with`` context, it is not
possible to access the file contents outside of the scope of that context,
because any memory mapped arrays will no longer be available.

It may sometimes be useful to copy array data into memory instead of using
memory maps. This can be controlled by passing ``memmap=False`` to either
the `AsdfFile` constructor or `asdf.open`. By default, ``memmap=True``.

0 comments on commit 231197c

Please sign in to comment.