Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Child classes do not call the __init__ method of the base class #2378

Open
DimitriPapadopoulos opened this issue Oct 15, 2024 · 2 comments
Open
Labels
bug Potential issues with the zarr-python library

Comments

@DimitriPapadopoulos
Copy link
Contributor

DimitriPapadopoulos commented Oct 15, 2024

Zarr version

v3

Description

I think this could and should be fixed in most cases:

Do you agree?

Steps to reproduce

Full list of occurrences available here:

@DimitriPapadopoulos DimitriPapadopoulos added the bug Potential issues with the zarr-python library label Oct 15, 2024
@jhamman
Copy link
Member

jhamman commented Oct 15, 2024

storage.logging.LoggingStore.init does not call abc.store.Store.init

This is the expected behavior. The LoggingStore wraps another Store object and the assumption is that this store called super().__init__(). I don't think it would make sense to call it again.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Oct 16, 2024

While the child class does not have to call super.__init__(), it is usually expected to.

In this specific case, LoggingStore inherits Store. If possible, LoggingStore.__init__ should call Store.__init__. That does seem possible and preferable here, as the base class initialises some private variables in Store.__init__:

        self._is_open = False
        self._mode = AccessMode.from_literal(mode)

which the child class does not initialise in LoggingStore.__init__:

        self._store = store
        self.counter = defaultdict(int)
        self.log_level = log_level
        self.log_handler = log_handler

        self._configure_logger(log_level, log_handler)

Shouldn't the variables of the base class be initialised when creating a child class? I may be missing something, but then it could mean that the code is overly complex, breaking usual assumptions without any visible explanation.

Said otherwise, why have a non-empty __init__ in an abstract base class, if it is not called by child classes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Potential issues with the zarr-python library
Projects
None yet
Development

No branches or pull requests

2 participants