Commit 24df678
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- nibabel
- freesurfer
- tests
3 files changed
+8
-43
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
454 | 454 | | |
455 | 455 | | |
456 | 456 | | |
457 | | - | |
458 | | - | |
459 | 457 | | |
460 | 458 | | |
461 | 459 | | |
462 | 460 | | |
463 | | - | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
464 | 464 | | |
465 | 465 | | |
466 | 466 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
149 | 149 | | |
150 | 150 | | |
151 | 151 | | |
152 | | - | |
| 152 | + | |
153 | 153 | | |
154 | | - | |
155 | | - | |
156 | | - | |
| 154 | + | |
| 155 | + | |
157 | 156 | | |
| 157 | + | |
158 | 158 | | |
159 | | - | |
160 | | - | |
161 | | - | |
162 | | - | |
163 | | - | |
164 | | - | |
165 | | - | |
166 | | - | |
167 | | - | |
168 | | - | |
169 | | - | |
170 | | - | |
171 | | - | |
172 | | - | |
173 | | - | |
174 | | - | |
175 | | - | |
176 | | - | |
177 | | - | |
178 | | - | |
179 | | - | |
180 | | - | |
181 | | - | |
182 | | - | |
183 | | - | |
184 | | - | |
185 | | - | |
186 | | - | |
187 | | - | |
188 | | - | |
189 | | - | |
190 | | - | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
115 | 115 | | |
116 | 116 | | |
117 | 117 | | |
118 | | - | |
119 | | - | |
120 | | - | |
| 118 | + | |
121 | 119 | | |
122 | 120 | | |
123 | | - | |
124 | 121 | | |
125 | 122 | | |
126 | 123 | | |
| |||
0 commit comments