Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions gridData/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ class Grid(object):

def __init__(self, grid=None, edges=None, origin=None, delta=None,
metadata=None, interpolation_spline_order=3,
file_format=None):
file_format=None, **kwargs):
Copy link
Copy Markdown
Member

@BradyAJohnston BradyAJohnston Jan 14, 2026

Choose a reason for hiding this comment

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

I think I would prefer an actual argument rather than **kwargs as the latter can easily (and silently) absorb spelling mistakes fiel_format='MRC would be silently no longer passed on instead of raising an error otherwise. I defer to @orbeckst on this but I think just mentioning in the docs that it is only used for mrc files (and maybe a warning when used for non-compatible format?).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You're right that **kwargs can silently swallow typos. On the other hand, tracking individual modifier keywords for different parsers can get annoying, at least in principle. In MDAnalysis we've typically been using the approach of named keywords for arguments that universally apply and kwargs for modifiers.

GDF really does not see a flurry of development activity so the maintainability argument that I am raising is not particularly strong and I do see the value of being explicit and more robust for users. So then let's follow your idea and make is_volume a top-level kwarg of Grid, document it there and in MRC (with a proper versionadded), and propagate it explicitly.

Copy link
Copy Markdown
Member

@BradyAJohnston BradyAJohnston Jan 14, 2026

Choose a reason for hiding this comment

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

I feel strongly about it mostly because I am endlessly spending hours bug fixing when it was just me with simple spelling mistakes.

# file formats are guessed from extension == lower case key
self._exporters = {
'DX': self._export_dx,
Expand Down Expand Up @@ -238,7 +238,7 @@ def __init__(self, grid=None, edges=None, origin=None, delta=None,
filename = str(grid)

if filename is not None:
self.load(filename, file_format=file_format)
self.load(filename, file_format=file_format, **kwargs)
else:
self._load(grid, edges, metadata, origin, delta)

Expand Down Expand Up @@ -532,7 +532,7 @@ def _load(
"grid={0} edges={1} origin={2} delta={3}".format(
grid, edges, origin, delta))

def load(self, filename, file_format=None):
def load(self, filename, file_format=None, **kwargs):
"""Load saved grid and edges from `filename`

The :meth:`load` method calls the class's constructor method and
Expand All @@ -545,32 +545,32 @@ def load(self, filename, file_format=None):
# are not really a file
raise IOError(errno.ENOENT, "file not found", filename)
loader = self._get_loader(filename, file_format=file_format)
loader(filename)
loader(filename, **kwargs)

def _load_python(self, filename):
def _load_python(self, filename, **kwargs):
with open(filename, 'rb') as f:
saved = pickle.load(f)
self._load(grid=saved['grid'],
edges=saved['edges'],
metadata=saved['metadata'])

def _load_mrc(self, filename):
def _load_mrc(self, filename, **kwargs):
"""Initializes Grid from a MRC/CCP4 file."""
mrcfile = mrc.MRC(filename)
mrcfile = mrc.MRC(filename, **kwargs)
grid, edges = mrcfile.histogramdd()
self._load(grid=grid, edges=edges, metadata=self.metadata)
# Store header for access from Grid object (undocumented)
# https://github.com/MDAnalysis/GridDataFormats/pull/100#discussion_r782604833
self._mrc_header = mrcfile.header.copy()

def _load_dx(self, filename):
def _load_dx(self, filename, **kwargs):
"""Initializes Grid from a OpenDX file."""
dx = OpenDX.field(0)
dx.read(filename)
grid, edges = dx.histogramdd()
self._load(grid=grid, edges=edges, metadata=self.metadata)

def _load_plt(self, filename):
def _load_plt(self, filename, **kwargs):
"""Initialize Grid from gOpenMol plt file."""
g = gOpenMol.Plt()
g.read(filename)
Expand Down
19 changes: 15 additions & 4 deletions gridData/mrc.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ class MRC(object):
----------
filename : str (optional)
input file (or stream), can be compressed
is_volume : bool (optional)
Force input file to be interpreted as a volume. If None (default),
use the MRC file header. If True, force the file to be read as a
volume. If False, the file is intepreted as a 2D image stack and
raise a ValueError.


Raises
------
Expand Down Expand Up @@ -86,17 +92,22 @@ class MRC(object):

"""

def __init__(self, filename=None):
def __init__(self, filename=None, is_volume=None):
self.filename = filename
if filename is not None:
self.read(filename)
self.read(filename, is_volume)

def read(self, filename):
def read(self, filename, is_volume=None):
"""Populate the instance from the MRC/CCP4 file *filename*."""
if filename is not None:
self.filename = filename
with mrcfile.open(filename) as mrc:
if not mrc.is_volume(): #pragma: no cover
if is_volume is None:
is_volume = mrc.is_volume()
elif is_volume:
# non 3D volumes should always fail, regardless of is_volume value
is_volume = mrc.data is not None and len(mrc.data.shape) == 3
if not is_volume: #pragma: no cover
raise ValueError(
"MRC file {} is not a volumetric density.".format(filename))
self.header = h = mrc.header.copy()
Expand Down
1 change: 1 addition & 0 deletions gridData/tests/datafiles/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
DX = importlib_resources.files(__name__) / 'test.dx'
DXGZ = importlib_resources.files(__name__) / 'test.dx.gz'
CCP4 = importlib_resources.files(__name__) / 'test.ccp4'
ISPG_0 = importlib_resources.files(__name__) / "ispg_0.mrc"
# from http://www.ebi.ac.uk/pdbe/coordinates/files/1jzv.ccp4
# (see issue #57)
CCP4_1JZV = importlib_resources.files(__name__) / '1jzv.ccp4'
Expand Down
Binary file added gridData/tests/datafiles/ispg_0.mrc
Binary file not shown.
11 changes: 11 additions & 0 deletions gridData/tests/test_mrc.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ def test_triclinic_ValueError():
"supported, not"):
Grid(datafiles.MRC_EMD3001, file_format="MRC")

def test_mrcfile_volume_check():
with pytest.raises(ValueError, match="is not a volumetric density"):
Grid(datafiles.ISPG_0)

def test_mrcfile_volume_force():
Grid(datafiles.ISPG_0, is_volume=True)

def test_mrcfile_force_fail():
with pytest.raises(ValueError, match="is not a volumetric density"):
Grid(datafiles.CCP4_1JZV, is_volume=False)

class TestGridMRC():
@pytest.fixture(scope="class")
def grid(self):
Expand Down