arf2bark fails with ARF files created by ArfFormat openephys plugin

[dmeliza]

The issue is that the plugin creates a name attribute that conflicts with the name formal parameter of the bark.create_entry function.

FYI, in general ARF attributes should be namespaced (e.g. openephys_name) to avoid this kind of issue.

[gfetterman]

Ad-hoc fixes like the one suggested in #48 could become unwieldy as they pile up. I'd like to avoid them if possible. But I agree that a collision between the create_entry arguments and the contents of the attributes dict being passed is concerning, and constitutes a bug in bark regardless of what the open ephys ArfFormat module should have done.

I see two solutions that seem appropriate. (I'm trying to keep these as light as possible, to avoid breaking scripts that depend upon the existing signature of create_entry.)

1. Run a small script to perform this name mangling on your already-collected ARF data before running arf2bark. (I.e., mold the data to fit the format.)
2. Have arf2bark (and perhaps, as needed, other *-to-bark converters) mangle such attribute names without assuming why the collisions occur. It'd be done according to a defined rule (simplest: append increasing numbers until conflicts disappear), which could be overridden by an optional command-line parameter namespace.

Option 2 is a bit more work, but I like it better, and I can have something in place by tomorrow.

[dmeliza]

Agreed that #48 is a bit too ad hoc.

The collisions are an unfortunate consequence of the way python handles named parameters and destructuring, I guess. Thanks for offering to work on a fix right away.

[dmeliza]

On the other hand, the only collisions you have to guard against are ones that conflict with positional arguments, so the proposed fix in #48 is not that ad hoc. One could give users an option of what namespace to use, of course.

On Jun 4, 2018, at 18:02, Graham Fetterman notifications@github.com wrote:

Ad-hoc fixes like the one suggested in #48 could become unwieldy as they pile up. I'd like to avoid them if possible. But I agree that a collision between the create_entry arguments and the contents of the attributes dict being passed is concerning, and constitutes a bug in bark regardless of what the open ephys ArfFormat module should have done.

I see two solutions that seem appropriate. (I'm trying to keep these as light as possible, to avoid breaking scripts that depend upon the existing signature of create_entry.)

Run a small script to perform this name mangling on your already-collected ARF data before running arf2bark. (I.e., mold the data to fit the format.) Have arf2bark (and perhaps, as needed, other *-to-bark converters) mangle such attribute names without assuming why the collisions occur. It'd be done according to a defined rule (simplest: append increasing numbers until conflicts disappear), which could be overridden by an optional command-line parameter namespace. Option 2 is a bit more work, but I like it better, and I can have something in place by tomorrow.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[gfetterman]

The more I think about this, the less happy I am with the options - everything seems ad hoc, barring significant modifications to core bark functions. (Or some decorator sleight-of-hand that will probably be too clever for its own good.)

It seems like the principled way to avoid these collisions is to replace the usage of arbitrary keyword arguments as attributes throughout the core bark functions with passing a dict directly.

I'm going to continue with my original plan, but perhaps using a dict rather than keyword arguments is something to contemplate for 0.3. I wonder if @kylerbrown has an opinion about this?

[kylerbrown]

@gfetterman parameters probably shouldn't be passed around as keyword arguments. I think that creating a branch to fix this issue long term is great idea, and 0.3 seems very reasonable.