Skip to content

Conversation

@mscheltienne
Copy link
Member

@mscheltienne mscheltienne commented Sep 17, 2021

As discussed in #9752; a quick test of what would break if we change Info from a dict to a Bunch.
I gave it a quick test and the copy() method fails. copy.deepcopy() doesn't bring the set attributes and raises AttributeError: 'Info' object has no attribute 'ch_names'. Let's see if anything else breaks.

For the copy() method, one quick solution would be to leave the only property ch_names in the code as it is now, but I think it might bite us later on.

If not too much code breaks, let's put some more thoughts into Bunch and how to copy it to make sure it will work as intended.

@mscheltienne
Copy link
Member Author

Notes following those 2 very quick tests: it doesn't look to me like it's going to be a pain to add.

Bunch from MNE

  • Breaks Info.copy() method; deepcopy doesn't copy attributes and since I removed the .ch_names property, copied Info are missing Info.ch_names.

Bunch from scikit-learn:

  • Breaks AssertionError: assert 'FIFFV_SSS_JOB_NOTHING' in ['__class__', '__contains__', '__delattr__', '__delitem__', '__dict__', '__dir__', ...] in mne.io.tests.test_constants
  • Breaks Info(read_hdf5(fname_h5)) with TypeError: __init__() takes 1 positional argument but 2 were given. This is because I removed *args from Info initialization as I didn't see how it was fitting in.

I will pick this up next week!

@mscheltienne
Copy link
Member Author

Messed up CLI GitHub while reverting the 2 test changes.. Opening a new PR with dictionary keys exposed as attributes working for mne.Info as soon as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant