[Bug]:

[onceone]

Describe the bug

AnomalibDataset inherits from ABC. Why is there no abstractmethod decorator in it

Dataset

N/A

Model

N/A

Steps to reproduce the behavior

/

OS information

OS information:

• OS: [e.g. Ubuntu 20.04]
• Python version: [e.g. 3.10.0]
• Anomalib version: [e.g. 0.3.6]
• PyTorch version: [e.g. 1.9.0]
• CUDA/cuDNN version: [e.g. 11.1]
• GPU models and configuration: [e.g. 2x GeForce RTX 3090]
• Any other relevant information: [e.g. I'm using a custom dataset]

Expected behavior

/

Screenshots

No response

Pip/GitHub

pip

What version/branch did you use?

No response

Configuration YAML

/

Logs

/

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[samet-akcay]

@onceone, The ABC inheritance ensures that this class cannot be instantiated directly if it has any abstract methods, but currently it doesn't have any. It is an indication that, AnomalibDataset is the base class, and cannot be used itself.

We could abstractmethod for the methods that we want to force subclasses to override specific methods.

In which part of the implementation do you think abstractmethod is missing?

[alexriedel1]

The methods in AnomalibDataset aren't very abstract buit very concrete actually. They aren't even implemented in most child classes, so abstract methods wouldn't make sense.

I don't know why the choice was made, to make AnomalibDataset an abstract base class. Maybe there was a time when the child classes needed to implement methods.

EDIT: ah @samet-akcay already gave the answer..

[onceone]

Since there are no abstract methods, AnomalibDataset can be instantiated（as follows code), so why do we still need to inherit ABC？

from abc import ABC,abstractmethod
class Animal(ABC):

    # @abstractmethod
    def eat(self):
        print('eassst')

    @property
    def name(self):
        return 'Animal'
a = Animal()
a.eat()

[samet-akcay]

I don't exactly remember the main motivation, but I think we wanted to emphasise that AnomalibDataset cannot be used on its own yet since it is still missing __getitem__ method to be implemented.

@djdameln, can you comment on this?

[djdameln]

In previous versions of the library, AnomalibDataset had the abstract method _setup which had to be implemented in the subclasses to assign the samples dataframe. In v1.0.0 we stepped away from this design and assign the dataframe in the constructor. So you are right that it is technically no longer necessary to inherit from ABC. On the other hand, as @samet-akcay pointed out, the AnomalibDataset is not meant to be instantiated directly, and defining it as a base class could emphasise this.

[onceone]

I think I got it . Thank you. By the way, do you have any recommendations for books on software design? Thanks again