Wrong error in Metric.__iter__

[ValerianRey]

🐛 Bug

In the class Metric, the __iter__ method is defined as follows:

def __iter__(self):
    """Iteration over metrics are not allowed. Use metric collections for nesting metrics."""
    raise NotImplementedError("Metrics does not support iteration.")

In python's docs, the following note is written about NotImplementedError (https://docs.python.org/3/library/exceptions.html#NotImplementedError):

It should not be used to indicate that an operator or method is not meant to be supported at all – in that case either leave the operator / method undefined or, if a subclass, set it to None.

In fact, the use cases for NotImplementedError are:

In user defined base classes, abstract methods should raise this exception when they require derived classes to override the method, or while the class is being developed to indicate that the real implementation still needs to be added.

In PyCharm (and maybe other python code checkers), this leads to a warning for every sub-class of Metric, saying that all abstract methods should be implemented (PyCharm understands a method that raises a NotImplementedError as abstract, even if there is no @abstractmethod decorator on this method).

Was there a good reason to define __iter__ like that? Otherwise, could we remove it?

[ValerianRey]

Since __getitem__ is defined, we need to specifically disable __iter__, so I understand why it was defined like that. I think the correct way to define it is to simply set:

__iter__ = None

in Metric.

Apparently it's possible to do that since python 3.6: https://docs.python.org/3/whatsnew/3.6.html#other-language-changes
They specifically take __iter__ as an example:

It is now possible to set a special method to None to indicate that the corresponding operation is not available. For example, if a class sets __iter__() to None, the class is not iterable. (Contributed by Andrew Barnert and Ivan Levkivskyi in bpo-25958.)