[BUG] _get_class_flags dynamically slices the MRO index, permanently dropping tags for Mixins during multiple inheritance

[vedant21-oss]

Describe the bug
The core component _get_class_flags iterates over reversed(inspect.getmro(cls)[:-2]). The arbitrary [:-2] cut off removes the last two members of the object tree. In multiple-inheritance situations (e.g., class CustomAlgorithm(BaseObject, ArbitraryMixin)), ArbitraryMixin slides against the end index of the MRO. Consequently, its associated _tags and _flags properties get unconditionally deleted out of get_class_flags.

To Reproduce

import inspect
from skbase.base import BaseObject

class MyMixin:
    _tags = {"mixin_tag": "should_be_seen"}

# With multiple inheritance
class MyObj(BaseObject, MyMixin):
    _tags = {"obj_tag": "seen"}

tags = MyObj.get_class_tags()
print("Tags:", tags) # mixin_tag will be missing completely from the output due to the [:-2] slice logic

Expected behavior
Tags defined in all class ancestor mixins should be exposed regardless of multiple inheritance order or length (except for Python's base object which never contains flags).

Proposed Fix
Revert to iterating through the getmro() resolution tree entirely natively while explicitly discarding object rather than generic indices:

for parent_class in reversed(inspect.getmro(cls)):
    if parent_class is object:
        continue
    # ... check hasattr

[fkiraly]

and where exactly is this a problem?

We know what the last two items are, namely BaseObject and _FlagManager. Mixins should not be above BaseObject in the inheritance order.

This sounds like an AI hallucinated problem.

[fkiraly]

If this is not AI generated, can you kindly explain what use case you have? Do you have one?

[vedant21-oss]

Hi @fkiraly - thanks for the quick response. I understand the skepticism; let me give you a concrete reproduction to show this is a real bug, not a hallucinated one.

Reproduction

import inspect
from skbase.base import BaseObject

class MyMixin:
    _flags = {"mixin_tag": 42}

# Multiple inheritance: BaseObject listed FIRST, so MyMixin ends up LAST in MRO
class MyObj(BaseObject, MyMixin):
    _flags = {"own_tag": 1}
    def __init__(self):
        super().__init__()

print("Full MRO:", [c.__name__ for c in MyObj.__mro__])
print("MRO[:-2]:", [c.__name__ for c in MyObj.__mro__[:-2]])
print()
print("Collected flags:", MyObj._get_class_flags())
print("'mixin_tag' present?", "mixin_tag" in MyObj._get_class_flags())

Output on unpatched main:

Full MRO: ['MyObj', 'BaseObject', '_FlagManager', 'MyMixin', 'object']
MRO[:-2]: ['MyObj', 'BaseObject', '_FlagManager']

Collected flags: {'own_tag': 1}
'mixin_tag' present? False   <- BUG: MyMixin's flag silently dropped

Root Cause

You're right that BaseObject and _FlagManager are the last two entries for simple inheritance. But Python's C3 linearisation changes the order under multiple inheritance. When a user writes class MyObj(BaseObject, MyMixin), the MRO becomes:

MyObj -> BaseObject -> _FlagManager -> MyMixin -> object

MyMixin lands at position [-2] - exactly where the slice cuts off. The [:-2] assumption only holds when no user-defined class appears after _FlagManager.

The Fix

Replace the hardcoded slice with an explicit sentinel check:

# Before (fragile - assumes MRO[-2] is always a framework class)
for parent_class in reversed(inspect.getmro(cls)[:-2]):

# After (safe - only skips the one class that can never have user flags)
for parent_class in reversed(inspect.getmro(cls)):
    if parent_class is object:
        continue

object is the only truly safe sentinel since it is always last, always present, and can never carry user-defined flags.

Use Case

Any library user who mixes in a custom Mixin with its own _flags alongside BaseObject will silently lose those flags - no error, no warning, just wrong runtime behaviour. This is a correctness bug in the flag collection logic.

I've opened a PR with the fix + regression test here: https://github.com/vedant21-oss/skbase/compare/fix/539-mro-slice-drops-mixin-flags

[fkiraly]

alternative suggestion: adding a test to TestAllObjects - the default test suite - to check that indeed BaseObject is the last in the MRO?

[vedant21-oss]

Hey @fkiraly, the explicit iteration safely handles cases where users write class MyEstimator(BaseObject, MyMixin).

Python's C3 linearisation pushes MyMixin to index -2 (right after _FlagManager), so slicing with [:-2] accidentally drops the custom Mixin entirely:

class MyEstimator(BaseObject, MyMixin): pass
print(MyEstimator.__mro__)
# Ends with: ... _FlagManager'>, <class '__main__.MyMixin'>, <class 'object'>)

Looping the full MRO and only skipping the object sentinel ensures tags aren't quietly lost.

(Also, I've removed the verbose comment you mentioned and added an MRO regression test to TestAllObjects!)