Image list cleanup

[zas]

Summary

• This is a…
  • Bug fix
  • Feature addition
  • Refactoring
  • Minor / simple change (like a typo)
  • Other
• Describe this change in 1-2 sentences:

Problem

• JIRA ticket (optional): PICARD-XXX

Solution

Action

Additional actions required:

• Update Picard documentation (please include a reference to this PR)
• Other (please specify below)

[phw]

@zas I started reviewing, but I'm not through it yet. But so far this looks really good.

About the issue with Item: I actially suspect that initial intention was indeed that Item is more UI related. But it didn't turn out this way. For the UI part there are the TreeItem classes.

The Item class gained stuff over the years. And this shows another oddity: I don't think the DataObj class has any reason to exist. It looks like this was kind of the non-UI base class. But I think the stuff there should actually be put into Item or your MetadataItem class. That way we also avoid the awkward multiple inheritance for many of the classes, without clear distinction what does what.

We can do this in a separate refactoring, but here is my short comment on the DataObj members:

class DataObject(LockableObject):

    def __init__(self, obj_id):
        super().__init__()
        self.id = obj_id  # Probably best placed in MetadataItem
        self.item = None  # I think this should go to Item, likely with a different name (ui_item ?)
        self._genres = Counter() # -> MetadataItem

    @property
    def genres(self): # -> MetadataItem
        ...

    def add_genre(self, name, count): # -> MetadataItem
        ...

    @staticmethod
    def set_genre_inc_params(inc, config=None): # -> MetadataItem
        ...

[zas]

I don't think the DataObj class has any reason to exist.

I agree, let's first merge this and then we can fix that. This PR is big enough.

[phw]

This is a great move. While reviewing the commits I had a couple of comments, but they were all addressed later on.

I'd be in favor of avoiding inline declaration. I think it harms readability. Also the type is recreated on every function call, which seems wasteful.

Apart from that LGTM.