Support for BrainVoyager VTC, MSK, VMR and NR-VMP files #216
Conversation
|
Thanks a lot - it would be good to support these files. We really need tests - the problem is that soon becomes very hard to alter the code if we can't confirm it is still working - and it's also much harder to see how the code works without tests. Can you find some very small (< 30K) example files to test against - and include in the repo? |
|
I will compile some test files and include them. I will also try to dabble in nose tests. Good to hear that you guys appreciate the new features :) |
|
OK, I added three test files (one for each format) that show up correctly in BrainVoyager. BTW: The data are just noise, so do not expect any brain-like images. |
|
Sorry, I just got to look at this one. Generally the code looks nicely done, thank you. The striking issue is the lack I personally prefer to put my constants like the data definitions for the header You may want to look at You've very sensibly added 'order' to the ArrayProxy init - in fact I did the Those were the thoughts that came to me from a quick read - thanks. |
|
Thanks for your comments! I refactored the ArrayProxy implementation. Regarding the constants: When I started writing the code I was of course inspired by analyze.py. In there the constants are kept on a global level. I like that and it looks much cleaner, indeed. However, BrainVoyager file format have almost always variable-length headers so the constants are no real constants anymore. Would you have a suggestion for a clean solution to that? I now look at the tests! |
|
Thanks for this work - I like the changes. I don't know if it is worth copying so much of the 'from_header' class method By the way, I can't see 'as_bv_map' in the code. Do you need 'get_slope_inter', 'set_slope_inter'? For the header object? Can you point to some documentation for the format somewhere? Doesn't 'mask_header_dtd' belong at the module global level? How about putting 'vmp_header_dtd' at the module global level, and the submap I guess you could keep a template for 'vtc_header_dtd' and fill in the 'prts' |
|
Thanks again for your comments! OK, now the 'as_analyze_map' makes more sense for me! Actually it was still a left-over from adapting analyze.py. I have not implemented 'as_bv_map' myself. I might get rid of it for now, however, it might something nice for having an inter-compatibility between BV filetypes like you suggested. I actually do not need get/set_slope_inter for BV filetype. This is also just an adaptation left-over. I will get rid of it. There is some documentation: I will try to refactor the dtype templates so that they reside at the modules global level. I had a look at test_image_api.py and tried to write some tests. I ran into problems regarding the dtypes - I need to fix the function of get/set_data_dtype. I am on it. |
|
Thanks a lot for the links. Did you already put them somewhere in the code
If you find you're having to do dumb stuff to get BV format to work we've On Tue, Dec 17, 2013 at 8:20 AM, Thomas Emmerling
|
|
Anything I can do to help here? |
|
Sorry for not responding in a while - I have been travelling and only came back two days ago. I will continue to work on the pull request at the end of this week. |
|
Looking forward to this. Let me know if you need help, I know Brainvoyager and Python. Also you might want to checkout NeuroElf, the matlab API for brainvoyager files, see how they do this. |
|
OK, this is not abandoned - I had some other work to do and it took me some time to figure out how some things in nose tests work.
I will also write more format-specific tests (test_bv.py). I did some major refactoring to put the header dtype definition at the modules top level (actually putting it into functions in some cases). Indeed, I think this helps readability a lot! I also tried to inherit more from the main BvFileHeader and BvFileImage classes to unify things more. I put the link to file format informations in the doctstring of each module. I also got my head around affine transformations for BV files. This document provides quite some information on how axes in BV files are aligned: http://support.brainvoyager.com/documents/Available_Tools/Available_Plugins/niftiplugin_manual_180610.pdf Have I understood it right that the affine transformation should produce data in RAS orientation? So that the first axis has values left-to-right (from subjects perspective), the second is posterior-to-anterior, and the third is inferior-to-superior? Are there more things to take into account? Could you hint me to a straightforward way of visualising such a transform with sample data to compare it to the way that BrainVoyager displays it? @ilogue Thank you for the hint to NeuroElf! Indeed, I use it on a daily basis. However, MATLAB can feel like a cage if you know the world of Python... ;) |
|
Would you have any new comments for me? |
|
Ah very sorry to be slow - I have something I have to finish these next few On Tue, Mar 11, 2014 at 12:37 AM, Thomas Emmerling <notifications@github.com
|
|
No problem, take your time! |
|
Sorry for bumping this again, but would you have a look and some comments to my open questions? |
|
A thousand apologies for being so slow - I will have a look now. |
|
OK - I see my fear that I would have to do some serious reading was justified :) I'm starting to grasp that you want to support three or four of the many BV formats:
I will get down to more reading - but it would help a lot if you had time to spend 10 minutes or so on Skype to talk me through it. Is that possible? If you think you might have time, I'll suggest a developer hangout for nibabel to get things moving a little quicker. |
nibabel/bv_vtc.py
Outdated
| from .spatialimages import HeaderDataError, HeaderTypeError | ||
| from .batteryrunners import Report | ||
|
|
||
| def _make_vtc_header_dtd(fmrlt,prts): |
There was a problem hiding this comment.
Would it be neater to specify an int length, tuple of int lengths here? Then add the 'S" at the beginning for the field definition, and the field names for the prts? That way you wouldn't have to do that in the calling code.
|
Thank you for your comments! |
|
How about tomorrow at 2000 your time 1100 my time? Meanwhile I will do some documentation of the image format. Do you mind if I invite anyone on the list who wants to join? |
|
That sounds great! I just sent you an email with my contact details. See you tomorrow! |
|
I was thinking, that you could define a new format for your header information - maybe a ordered dict, and pass this format into the constructor, as in: The VTCHeader class would implement I guess you could just use the |
|
Alternatively you could have the header being contained by a numpy structured array with the stuff that is fixed, and a dictionary for the strings that are not - such as the strings. Maybe then you wouldn't need the IPython traitlets. |
|
I thought a lot about the different ways there are to make the code more clean. However, they all have their drawbacks:
I am just not sure if the final version would be any more readable than right now. Do you share my concerns? ;) |
|
Is it possible to read VTC files using nibabel at this point? |
|
@mtakemiya Yes that should be possible with the fork on my github account. Indeed, I used it for my own data. However, it is not thoroughly tested yet. |
|
For the format - are these the main problems?
At some point the data has to be written out in binary format, and its convenient to use the numpy structured array format to do that. The question is whether that's the best way to store the header data - and read it in. For example, I think I'm right that - at the moment - it's not easy to add a file to the list of PRT files? Or change a name? Maybe I'm not understanding. To make it easier to do something like that, I could imagine encoding the string data - say as python strings, recorded as 'O' (object) type in the structured array defining the header. Similarly the PRT files could be of 'O' type, but this time be a list. For changing things in memory, this would just work as expected, appending to a list, changing the string for another string. For writing you could convert the 'O' field on the fly to to an S field, or the list of strings to a sequence of S fields - I guess. I mean - use a different structured array for storing in memory and writing out to files. But - you have a much better feeling for the problem than me, so go with what you think is best. |
|
Where are we with this one? I'm planning a release soon - maybe we should defer this one until the next release? I will do another release as soon as this is ready. You probably saw that the OrderedDict PR is merged now. I think I said that I would do a doc on coordinate systems, and that I would make a better image testing rig. I will get onto those now. Anything else I can help with? |
|
Sorry for the long delay - after HBM there were some project tasks to catch up with that took me quite some time. The new doc on coordinate systems is definitely helpful and the image testing would be great, too! Apart from that I will have look during the next week whether I can store data in a more convenient/pythonic way between reading and writing. I had already added some facilities to allow for example adding and removing of submaps in VMP files (that would need an on-the-fly header and corresponding structured numpy array change) but the solution is indeed not very clean. I hope I can finish that over the next week. However, maybe it is wiser to include it after the next release, also to allow more time for testing. |
thomastweets
changed the title
|
@matthew-brett OK, finally coverage increased (and is very high for the BV-related files). This is now definitely ready for review :) |
|
Thanks - I will try and get to this ASAP. |
|
@matthew-brett I know that this became quite a monster of a PR, but do you think it can be merged for the next release? Just to give you a heads-up: I will start a new job in September and might be (even) more unresponsive in the future unfortunately because of time constraints. |
|
Sorry to take a while to get back to you, I just had the first day of a course I am teaching, and I got a bit overwhelmed. Have time now, but, is it too late for you? Do you have a day or so to respond to stuff, or will you be maxed out? |
|
Now it took me a while to get back to you ;) |
nibabel/brainvoyager/bv.py
Outdated
| str_list : generator of string(s) | ||
| """ | ||
| currentPos = f.tell() | ||
| if strip: |
There was a problem hiding this comment.
Consider suffix = b'' if strip else b'\x00'
nibabel/brainvoyager/bv.py
Outdated
| ------- | ||
| str_list : generator of string(s) | ||
| """ | ||
| currentPos = f.tell() |
There was a problem hiding this comment.
PEP8 naming conventions current_pos for currentPos etc.
nibabel/brainvoyager/bv.py
Outdated
| if rewind: | ||
| f.seek(currentPos) | ||
| else: | ||
| offset = 0 |
There was a problem hiding this comment.
How about:
offsets = [len(lines[s]) + 1 for s in range(nStrings)]
f.seek(current_pos + sum(offsets))
nibabel/brainvoyager/bv.py
Outdated
| ) | ||
|
|
||
|
|
||
| def readCString(f, nStrings=1, bufsize=1000, startPos=None, strip=True, |
There was a problem hiding this comment.
Consider read_c_string for PEP8 naming?
nibabel/brainvoyager/bv.py
Outdated
| Parameters | ||
| ---------- | ||
| f : fileobj | ||
| File object to use |
There was a problem hiding this comment.
Mention that object should implement tell, seek, read.
nibabel/brainvoyager/bv.py
Outdated
| # handle conditional fields | ||
| elif isinstance(def_or_name, tuple): | ||
| if hdr_dict[def_or_name[1]] == def_or_name[2]: | ||
| bytes = fileobj.read(calcsize(format)) |
There was a problem hiding this comment.
This is a builtin for Python 3 - rename to bytes_ or similar?
nibabel/brainvoyager/bv.py
Outdated
| hdr_dict: OrderedDict | ||
| hdr_dict that contains the fields and values to for the respective | ||
| BV file format. | ||
| parent_hdr_dict: OrderedDict |
There was a problem hiding this comment.
Document None, and is optional, as for parse_BV_header above.
nibabel/brainvoyager/bv.py
Outdated
| 'NrOfLags' in the VMP file header). | ||
| """ | ||
| hdr_dict = OrderedDict() | ||
| for name, format, def_or_name in hdr_dict_proto: |
There was a problem hiding this comment.
format is a builtin function, rename to format_ or similar?
nibabel/brainvoyager/bv.py
Outdated
| hdr_dict: OrderedDict | ||
| hdr_dict that contains the fields and values to for the respective | ||
| BV file format. | ||
| parent_hdr_dict: OrderedDict |
There was a problem hiding this comment.
Document None, and that it's optional, as for parse_BV_header above?
| n_values = hdr_dict[def_or_name] | ||
| else: | ||
| n_values = parent_hdr_dict[def_or_name] | ||
| for i in range(n_values): |
There was a problem hiding this comment.
maybe
hdr_size = sum(calc_BV_header_size(format_, value[i], hdr_dict) for i in range(n_values))
There was a problem hiding this comment.
I changed all other comments (from this post) but here I run into failed tests when changing to this. I will still look into it once more, however, for now I left it like it was before.
There was a problem hiding this comment.
I can't follow my own logic here, so not changing is very reasonable.
| if hdr_dict[def_or_name[1]] == def_or_name[2]: | ||
| part = pack('<' + format, value) | ||
| else: | ||
| continue |
There was a problem hiding this comment.
Would you mind putting a comment here to explain this case?
nibabel/brainvoyager/bv.py
Outdated
| """ | ||
| for name, format, def_or_name in hdr_dict_proto: | ||
| # handle nested loop fields | ||
| if isinstance(format, tuple): |
There was a problem hiding this comment.
Should the format be ignored in this case? Maybe a comment here? To reduce indentation and make intention clear, mabye
if not isinstance(format, tuple):
continue
...
nibabel/brainvoyager/bv.py
Outdated
| if format == 'z': | ||
| hdr_size += len(value) + 1 | ||
| # handle array fields | ||
| elif isinstance(format, tuple): |
There was a problem hiding this comment.
This pattern seems to repeat a lot. Can you think of a way of making this more general, to share code between the functions with these same sets of conditionals? I was thinking vaguely about mapping these conditionals to actions with a dictionary or something that. Maybe a silly idea, asking in hope you can come up with something.
There was a problem hiding this comment.
I agree that this is not the most beautiful style here. However, there are a lot of weird edge cases with BV headers and I fear that while trying to make things more general one would make things really hard to understand, too. At least this repetition is confined to these module level functions in bv.py and do not repeat for different file formats.
I will give it some more thought though.
There was a problem hiding this comment.
Yes, I understand, and I agree, if nothing clean and nice comes to mind, the repetition is OK.
nibabel/brainvoyager/bv.py
Outdated
| hdr_dict before any changes. | ||
| hdr_dict_new: OrderedDict | ||
| hdr_dict with changed fields in n_fields_name or c_fields_name fields. | ||
| parent_old: OrderedDict |
There was a problem hiding this comment.
Note can be None, and is optional.
There was a problem hiding this comment.
As in:
parent_old: None or OrderedDict, optional
nibabel/brainvoyager/bv.py
Outdated
| When update_BV_header() is called recursively the not yet updated | ||
| (parent) hdr_dict is passed to give access to n_fields_name fields | ||
| outside the current scope (see below). | ||
| parent_new: OrderedDict |
There was a problem hiding this comment.
Can be None, is optional.
nibabel/brainvoyager/bv.py
Outdated
| if isinstance(format, tuple): | ||
| # calculate the change of array length and the new array length | ||
| if def_or_name in hdr_dict_old: | ||
| delta_values = hdr_dict_new[def_or_name] - \ |
There was a problem hiding this comment.
Vague preference for parentheses round expression to allow line breaks, rather than continuation character \ : https://www.python.org/dev/peps/pep-0008/#id19
nibabel/brainvoyager/bv.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| st_array: array |
There was a problem hiding this comment.
I guess this is a 3D array shape (n, 4, 4)?
| ------- | ||
| combined_st : array of shape (4, 4) | ||
| """ | ||
| if len(st_array) == 1: |
There was a problem hiding this comment.
How about:
from functools import reduce
if inv:
st_array = [np.linalg.inv(aff) for aff in st_array]
return reduce(np.dot, st_array)
There was a problem hiding this comment.
Any thoughts on suggested edit here?
There was a problem hiding this comment.
I can't remember where we were with the inverses and ordering? Is this right - to multiply all the affines in the order A[0] @ A[1] @ .... A[N-1] (where @ is matrix multiplication)? So A[N-1] applied first, A[0] applied last? And same for inverses?: inv(A[0]) @ inv(A[1]) @ ... inv(A[N-1])?
There was a problem hiding this comment.
Thomas - what do you think about this one? Is there any way to check what the order should be?
There was a problem hiding this comment.
And - what do you think of my suggested edit above?
| array filled with transformation matrices of shape (4, 4) | ||
|
|
||
| inv: boolean | ||
| Set to true to invert the transformation matrices before |
There was a problem hiding this comment.
This is a slightly odd transformation - can you comment more? I was expecting you to invert the result of the multiplication, not the individual elements.
Is it right that the transformation you want is st_array[-1] followed by st_array[-2] ... etc? I mean, to do mean the order of the arrays to be the order of the transformation, or the opposite (as you get from taking the dot product from left to right).
There was a problem hiding this comment.
OK, I am waiting for a connection flight right now that was delayed 5 hours and I am already jetlagged, so I should definitely not make up my mind about spatial transformation matrices, BUT:
What I want here is to compute a transformation matrix from the orientation of the data in the BV file to their native orientation by reversing all the transformations that were performed on the data. These transformations are stored (e.g.) in the VMR file header in an ordered fashion. So I guess if taking the dot product from left to right gets me there, this is what I want.
But again, I will better try this out with a clear mind :)
There was a problem hiding this comment.
So sorry for long delay - was teaching this term.
Nearly there now.
Have you got any other BV files lying around we can add as more extensive tests, using http://nipy.org/nibabel/devel/add_test_data.html#adding-as-a-submodule-to-nibabel-data ?
| hdr_dict_proto: tuple | ||
| tuple of format described in Notes below. | ||
| fileobj : fileobj | ||
| File object to use. Make sure that the current position is at the |
| n_values = hdr_dict[def_or_name] | ||
| else: | ||
| n_values = parent_hdr_dict[def_or_name] | ||
| for i in range(n_values): |
There was a problem hiding this comment.
I can't follow my own logic here, so not changing is very reasonable.
nibabel/brainvoyager/bv.py
Outdated
| hdr_dict before any changes. | ||
| hdr_dict_new: OrderedDict | ||
| hdr_dict with changed fields in n_fields_name or c_fields_name fields. | ||
| parent_old: OrderedDict |
There was a problem hiding this comment.
As in:
parent_old: None or OrderedDict, optional
| # handle only nested loop fields | ||
| if not isinstance(pack_format, tuple): | ||
| continue | ||
| else: |
There was a problem hiding this comment.
Can drop this else: and deindent the block after it.
| ------- | ||
| combined_st : array of shape (4, 4) | ||
| """ | ||
| if len(st_array) == 1: |
There was a problem hiding this comment.
Any thoughts on suggested edit here?
| [0., 0., 1., 1.], | ||
| [0., 0., 0., 1.]] | ||
| assert_array_equal(combinedST, correctCombinedST) | ||
| combinedST = combine_st(STarray, inv=False) |
There was a problem hiding this comment.
Is this test sensitive to order?
| with InTemporaryDirectory(): | ||
| # create a tempfile | ||
| path = 'test.header' | ||
| fwrite = open(path, 'wb') |
There was a problem hiding this comment.
If you can be bothered, nicer as:
with open(path, 'wb') as fwrite:
fwrite.write(binary)
Similarly for open usage below.
| @@ -281,7 +281,7 @@ def set_data_dtype(self, datatype): | |||
| try: | |||
| code = self._data_type_codes[datatype] | |||
| except KeyError: | |||
| raise MGHError('datatype dtype "%s" not recognized' % datatype) | |||
| raise HeaderDataError('datatype dtype "%s" not recognized' % datatype) | |||
There was a problem hiding this comment.
With the added supported_np_types check in spatialimages the missing HeaderDataError produced errors in the test_image_api and test_image_load_save tests. HeaderDataError seems to be the "correct" error to raise here.
| @@ -50,11 +50,6 @@ def copy(self): | |||
| return FunkyHeader(self.shape) | |||
|
|
|||
|
|
|||
| class CArrayProxy(ArrayProxy): | |||
There was a problem hiding this comment.
Did you mean to move this to the BV module?
There was a problem hiding this comment.
(I think based on your suggestion) I included the CArrayProxy class to arrayproxy.py because I need it for BV file formats. As this class was defined in test_arrayproxy.py as well I rather imported it and deleted the duplicate. But maybe I did not understand the logic here (or your question :)).
nibabel/tests/test_image_api.py
Outdated
| supported_dtypes = supported_np_types(img.header_class()) | ||
| if new_dtype not in supported_dtypes: | ||
| new_dtype = supported_dtypes.pop() | ||
| except: |
There was a problem hiding this comment.
Can you specify the expected errors here? Bare except is a bit frightening...
|
@thomastweets - will you have any time to respond to my comments here (final stretch)? Or would you prefer I merged this as-is, and then submitted some more PRs with edits? |
|
@matthew-brett I am very sorry for the late response: I am quite busy with my new job, however, I will try to make some time for your comments next weekend. Looking forward to having this completed :) |
|
@thomastweets - sorry to press you - will have any time to work on this? It's so close ... |
|
Sorry for taking so long! |
|
☔ The latest upstream changes (presumably #249) made this pull request unmergeable. Please resolve the merge conflicts. |
| ------- | ||
| combined_st : array of shape (4, 4) | ||
| """ | ||
| if len(st_array) == 1: |
There was a problem hiding this comment.
Thomas - what do you think about this one? Is there any way to check what the order should be?
| ------- | ||
| combined_st : array of shape (4, 4) | ||
| """ | ||
| if len(st_array) == 1: |
There was a problem hiding this comment.
And - what do you think of my suggested edit above?
| default_endianness = '<' # BV files are always little-endian | ||
| allowed_dtypes = [1, 2, 3] | ||
| default_dtype = 2 | ||
| allowed_dimensions = [3] |
There was a problem hiding this comment.
This attribute appears to be unused?
| endianness=default_endianness, | ||
| check=True, | ||
| offset=None): | ||
| """Initialize header from binary data block. |
There was a problem hiding this comment.
I think this doesn't initialize from a binary data block, but from a hdr_dict OrderedDict instance.
Am I right in think that, on order to know the length of a BV header, you need to parse the header? So it's not easy to pass in a given binary block to this routine, and then parse it, you have to parse it and then pass it in (here as hdr_dict).
In which case, this call isn't compatible with the Nifti1 etc header creation calls, and I think you can / should also drop the endianness parameter, because it's not doing anything at the moment - except getting checked.
| if header is None: | ||
| return obj | ||
| try: # check if there is a specific conversion routine | ||
| mapping = header.as_bv_map() |
There was a problem hiding this comment.
Sure. Each header defines its preferred map generation method, with default None. So:
Header.map_method_name == None
AnalyzeHeader.map_method_name = 'as_analyze_map'
BvFileHeader.map_method_name = 'as_bv_map'.
Then the Header class has the following method (moved from the AnalyzeHeader class):
@classmethod
def from_header(klass, header=None, check=True):
''' Class method to create header from another header
Parameters
----------
header : ``Header`` instance or mapping
a header of this class, or another class of header for
conversion to this type
check : {True, False}
whether to check header for integrity
Returns
-------
hdr : header instance
fresh header instance of our own class
'''
# own type, return copy
if type(header) == klass:
obj = header.copy()
if check:
obj.check_fix()
return obj
# not own type, make fresh header instance
obj = klass(check=check)
if header is None:
return obj
# Check for method that returns a mapping this header understands
map_method = (None if self.map_method_name is None
else getattr(self.header, self.map_method_name, None))
if map_method is not None:
# header is convertible from a field mapping
mapping = map_method()
for key in mapping:
try:
obj[key] = mapping[key]
except (ValueError, KeyError):
# the presence of the mapping certifies the fields as being
# of the same meaning as for this class, so we can
# safely discard fields with names not known to this header
# type on the basis they are from the wrong dialect
pass
# set any fields etc that are specific to this format (overriden by
# sub-classes)
obj._clean_after_mapping()
# Fallback basic conversion always done.
# More specific warning for unsupported datatypes
orig_code = header.get_data_dtype()
try:
obj.set_data_dtype(orig_code)
except HeaderDataError:
raise HeaderDataError('Input header %s has datatype %s but '
'output header %s does not support it'
% (header.__class__,
header.get_value_label('datatype'),
klass))
obj.set_data_dtype(header.get_data_dtype())
obj.set_data_shape(header.get_data_shape())
obj.set_zooms(header.get_zooms())
if check:
obj.check_fix()
return obj
This the variant where the method goes in the Header class, but it could also be a mixin class, just containing this method, where AnalyzeHeader and BvFileHeader both inherit from both of Header and HeaderMapMixin, as in:
class BvFileHeader(Header, HeaderMapMixin):
| if any([d > fc for d in (x, y, z)]): | ||
| continue | ||
| else: | ||
| return fc, fc, fc |
There was a problem hiding this comment.
What happens if some d > 1024 ?
|
|
||
| def get_data_offset(self): | ||
| """Return offset into data file to read data.""" | ||
| self.set_data_offset(calc_BV_header_size( |
There was a problem hiding this comment.
Your call. You can make set_data_offset raise an error if you like (parrec.py does this) - or you can let people do a set if they want, hoping they understand the risk.
Do people tend to store stuff in BV files after the header and before the data? That's not uncommon in Analyze, but very rare for Niftis for example. If it's rare, maybe raising an error is better.
By the way, you're using this in the header constructor, so I guess the offset in the header constructor is getting thrown away, in fact. Maybe you can remove it from the header parameter list?
| binaryblock = pack_BV_header(self.hdr_dict_proto, self._hdr_dict) | ||
| fileobj.write(binaryblock) | ||
|
|
||
| def check_fix(self, logger=None, error_level=None): |
There was a problem hiding this comment.
Well - could use a Mixin I guess - as above. Not sure whether it's worth it.
| @@ -281,7 +281,7 @@ def set_data_dtype(self, datatype): | |||
| try: | |||
| code = self._data_type_codes[datatype] | |||
| except KeyError: | |||
| raise MGHError('datatype dtype "%s" not recognized' % datatype) | |||
| raise HeaderDataError('datatype dtype "%s" not recognized' % datatype) | |||
|
@thomastweets Sorry to see this one fall by the wayside. Is there any interest in trying to pick it back up for the 2.5 (July) or 3.0 (October) releases? If you need help resolving conflicts, I'm happy to give it a go. |
After talking to Emanuele Olivetti and Yaroslav Halchenko about it I submit this pull request for implementing support for BrainVoyager QX files. I started out with VTC, MSK, and NR-VMP files and read and write does work. As this is my first major pull request I would really appreciate some comments or help. I did not write nose tests yet and the code is not thoroughly tested yet either.
I hope that my implementation is not too "dirty" as it was a little difficult for me to implement the variable-length header of BrainVoyager files...