NeuroM icon indicating copy to clipboard operation
NeuroM copied to clipboard

WIP: V4

Open mgeplf opened this issue 10 months ago • 2 comments

mgeplf avatar Apr 04 '24 12:04 mgeplf

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (a44ecc4) to head (69a0073).

:exclamation: Current head 69a0073 differs from pull request most recent head ab906d1. Consider uploading reports for the commit ab906d1 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            master     #1111    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           36        35     -1     
  Lines         2473      2610   +137     
==========================================
+ Hits          2473      2610   +137     

codecov-commenter avatar Apr 05 '24 09:04 codecov-commenter

The register/unregister in NeuriteType enum in this branch is quite fragile because it tries to bend the immutability of the python enums:

    @classmethod
    def register(cls, name, value):
        """Register a new value in the Enum class."""
        value = _int_or_tuple(value)

        if hasattr(cls, name):
            existing = getattr(cls, name)
            raise ValueError(f"NeuriteType '{name}' is already registered as {repr(existing)}")

        try:
            existing = cls(value)
        except ValueError:
            existing = None

        if existing:
            raise ValueError(f"NeuriteType '{name}' is already registered as {repr(existing)}")

        obj = _create_neurite_type(cls, value, name=name)

        cls._value2member_map_[value] = obj
        cls._member_map_[name] = obj
        cls._member_names_.append(name)

        return obj

    @classmethod
    def unregister(cls, name):
        """Unregister a value in the Enum class."""
        if name not in cls._member_names_:
            raise ValueError(
                f"The NeuriteType '{name}' is not registered so it can not be unregistered"
            )

        value = cls._member_map_[name].value
        del cls._value2member_map_[value]
        del cls._member_map_[name]
        cls._member_names_.remove(name)

Therefore, it is no surprise that some of the functionality we relied upon is now broken in 3.12 because they tried to make them safer: https://docs.python.org/3/whatsnew/3.12.html#enum

In addition, we rely on protected variables to make it work, which is also not great.

In the end, the use cases for which registration is needed are not so many. To make an int type usable, it has to also exist in MorphiIO as a SectionType and all section types are already available as neurite types.

That leaves us with registering new composite types that combine existing section types. As we don't have use cases where other type combinations are needed apart from axon-on-dendrite, which is already defined in NeuriteType, I suggest we remove the registration of neurite types and revisit them in the future when solid cases for this functionality appear.

eleftherioszisis avatar Apr 05 '24 11:04 eleftherioszisis