New deprecation wrapper for `Node.open` causes problems with `numpy`

[sphuber]

To prepare for the new repository interface in v2.0 the Node.open method wrapped the return value in the WarnWhenNotEntered class. However, this gives problems when used with numpy for example:

numpy.genfromtext(node.open('some_file'))

will raise a TypeError. The reason is because numpy will call iter on the file descriptor we pass, which is not implemented. We can easily implement this, but the question is what else have we missed? The idea of the wrapper was to prevent code from breaking when we go to v2.0, but is that really worth it if we break current code?

[sphuber]

@giovannipizzi and @greschd what do you think? Can we figure out all reasonable code pathways like these that we need to fix? Or should we maybe remove the wrapper?

[greschd]

I think where we went wrong is that special method lookup doesn't go through __getattr__, it's its own thing. Found this discussion: https://bugs.python.org/issue729913

[greschd]

Looking at the dunder methods of _io.TextIOWrapper, the relevant ones seem to be __next__ and __iter__.

[greschd]

It seems we rolled out "this will raise on AiiDA version x.y" a bit ahead of schedule 😅

[greschd]

Interestingly, open_file.__iter__() and open_file.__next__() work, but iter(open_file) and next(open_file) do not.

[greschd]

A quick recap: Adding the following

    def __iter__(self):
        return self._fileobj.__iter__()

    def __next__(self):
        return self._fileobj.__next__()

to the WarnWhenNotEntered class seems to resolve the immediate issue. I think this solves all relevant use cases, but can't be sure.

The "proper" way to ensure all magic methods are forwarded to the _fileobj would be inheriting from the right class. The problem there is that the type of the file object depends on the mode. So it would require a bit too much Python trickery to get that right in my opinion.

Of course the "safe" option is simply removing the wrapper - but then again, all cases where this is now broken will be broken again on 2.0 (inside context managers, everything works fine).

I'll leave that decision up to you @sphuber @giovannipizzi.

In any case, I think it warrants a quick bugfix release.

[sphuber]

In any case, I think it warrants a quick bugfix release.

Definitely, I already opened the milestone and added it. Will make new release once this is fixed together with other minor thing