Skip to content

Commit 24df678

Browse files
matthew-bretteffigies
authored andcommitted
RF: remove imageopener decorator
I was finding the decorator for ImageOpener confusing. Pros for the decorator: * Adding allowed `valid_exts` and specifying how it should be opened happen in the same place (decorator at top of class); * Using a decorator allows change of internal storage of the ImageOpener class. Cons: * Decorators can be hard to read, especially when the decorator is a function call (so the decorator returns a decorator); * It's hard to see at a glance which extensions are valid, because the decorator is at the top of the class, and adds later to `valid_exts`, whereas the apparent definition of `valid_exts` is after the docstring; * The decorator has to know about the internals of the class (in this case, the `valid_exts` attribute); * The role of the decorator is to signal that a certain extension has to be opened in a certain way, but it seems a bit odd that this signal also adds the extension to the class. To me it seems more natural to make this signal a line of its own, next to the `valid_exts` definition; * Modifying the ImageOpener dictionary directly doesn't stop us from making the dictionary a property or another object type, and changing the underlying storage; * No decorator means less code.
1 parent 9a67e76 commit 24df678

File tree

3 files changed

+8
-43
lines changed

3 files changed

+8
-43
lines changed

nibabel/freesurfer/mghformat.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -454,13 +454,13 @@ def writeftr_to(self, fileobj):
454454
fileobj.write(ftr_nd.tostring())
455455

456456

457-
# Register .mgz extension as compressed
458-
@ImageOpener.register_ext_from_image('.mgz', ImageOpener.gz_def)
459457
class MGHImage(SpatialImage):
460458
""" Class for MGH format image
461459
"""
462460
header_class = MGHHeader
463-
valid_exts = ('.mgh',)
461+
valid_exts = ('.mgh', '.mgz')
462+
# Register that .mgz extension signals gzip compression
463+
ImageOpener.compress_ext_map['.mgz'] = ImageOpener.gz_def
464464
files_types = (('image', '.mgh'),)
465465
_compressed_suffixes = ()
466466

nibabel/openers.py

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -149,42 +149,10 @@ def __exit__(self, exc_type, exc_val, exc_tb):
149149

150150

151151
class ImageOpener(Opener):
152-
""" Opener-type class passed to image classes to collect compressed extensions
152+
""" Opener-type class to collect extra compressed extensions
153153
154-
This class allows itself to have image extensions added to its class
155-
attributes, via the `register_ex_from_images`. The class can therefore
156-
change state when image classes are defined.
154+
A trivial sub-class of opener to which image classes can add extra
155+
extensions with custom openers, such as compressed openers.
157156
"""
157+
# Add new extensions to this dictionary
158158
compress_ext_map = Opener.compress_ext_map.copy()
159-
160-
@classmethod
161-
def register_ext_from_image(opener_klass, ext, func_def):
162-
"""Decorator for adding extension / opener_function associations.
163-
164-
Should be used to decorate classes. Updates ImageOpener class with
165-
desired extension / opener association. Updates decorated class by
166-
adding ```ext``` to ```klass.alternate_exts```.
167-
168-
Parameters
169-
----------
170-
opener_klass : decorated class
171-
ext : file extension to associate `func_def` with.
172-
should start with '.'
173-
func_def : opener function/parameter tuple
174-
Should be a `(function, (args,))` tuple, where `function` accepts
175-
a filename as the first parameter, and `args` defines the
176-
other arguments that `function` accepts. These arguments must
177-
be any (unordered) subset of `mode`, `compresslevel`,
178-
and `buffering`.
179-
180-
Returns
181-
-------
182-
opener_klass
183-
"""
184-
def decorate(klass):
185-
assert ext not in opener_klass.compress_ext_map, \
186-
"Cannot redefine extension-function mappings."
187-
opener_klass.compress_ext_map[ext] = func_def
188-
klass.valid_exts += (ext,)
189-
return klass
190-
return decorate

nibabel/tests/test_openers.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,9 @@ def file_opener(fileish, mode):
115115

116116
# Add the association
117117
n_associations = len(ImageOpener.compress_ext_map)
118-
dec = ImageOpener.register_ext_from_image('.foo',
119-
(file_opener, ('mode',)))
120-
dec(self.__class__)
118+
ImageOpener.compress_ext_map['.foo'] = (file_opener, ('mode',))
121119
assert_equal(n_associations + 1, len(ImageOpener.compress_ext_map))
122120
assert_true('.foo' in ImageOpener.compress_ext_map)
123-
assert_true('.foo' in self.valid_exts)
124121

125122
with InTemporaryDirectory():
126123
with ImageOpener('test.foo', 'w'):

0 commit comments

Comments
 (0)