Skip to content

Issue 95: Added PixelFormat class, replaced usages of color formats#121

Open
suppresseddev wants to merge 2 commits intomeetecho:mainfrom
suppresseddev:feature/static-class-pixel-colors
Open

Issue 95: Added PixelFormat class, replaced usages of color formats#121
suppresseddev wants to merge 2 commits intomeetecho:mainfrom
suppresseddev:feature/static-class-pixel-colors

Conversation

@suppresseddev
Copy link

Description

In reference to issue 95, I created the requested PixelFormat class and updated uses of pixel formatting with references to the PixelFormat class.

PR type
Select all the labels that apply to this PR.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring
  • Performance improvement
  • Test update
  • Build/CI configuration change
  • Other (please describe):

Key modifications and changes

Added juturna/names/_pixel_format.py, which contains the PixelFormat class.
Added the PixelFormat class to the package juturna.names.
Replaced usages of pixel formats with references to the PixelFormat class.

Affected components

Video Handling: Specifically, the things found under juturna/nodes/source

Additional context

N/A

@GaijinKa
Copy link
Member

Hey, thanks a lot for the contribution! The direction is right and the work is appreciated. 🙌

That said, I'd like to request a few changes before we can merge this.


1. Format normalization inside the class

The whole point of this class is to abstract away the differences between the various nodes — so consumers don't have to care about implementation details of each one. A couple of concrete cases already in the codebase:

  • YOLODetector uses hardcoded uppercase color format strings internally (e.g. "RGB", "BGR")
  • ImageLoader uses PIL's Image.convert(), which also expects uppercase mode strings (e.g. "RGB")

This inconsistency should be handled internally by the class rather than leaking out. I'd suggest adding a small normalization step — a private method that canonicalizes format strings before any comparison or dispatch:

@staticmethod
def _normalize_format(fmt: str) -> str:
    """Canonicalize format strings to a consistent internal representation."""
    return fmt.strip().lower()

Then use _normalize_format() consistently wherever format strings are compared or stored. This way, "rgb", "RGB", and " Rgb " all resolve to the same thing, and adding new nodes in the future stays frictionless.


2. Update the affected nodes accordingly

Both YOLODetector and ImageLoader should be updated to go through the new class rather than relying on their own hardcoded format strings. This ensures everything stays consistent and avoids parallel logic scattered across the codebase.


3. Consider using __slots__

Since this class is likely to be instantiated frequently (once per node, potentially many times during a pipeline run), it's worth declaring __slots__ on it.


4. Remove inline comments

Please strip out the inline comments — the code should be self-explanatory, and anything worth explaining belongs in the docstrings or the PR description. Comments that restate what the code does add noise without value.


5. Ruff linting is failing

The ruff workflow is currently failing on this PR. There are a few small things to fix — the exact rules and how to run the linter locally are documented in CONTRIBUTING.md. It should only take a few minutes. Please make sure the workflow is green before requesting a re-review.


These changes together would make the abstraction complete and robust. Happy to discuss any of them — feel free to ping me!

@GaijinKa
Copy link
Member

@suppresseddev Hi, are you still around? Have you had a chance to make any progress on this?

@suppresseddev
Copy link
Author

Oh I can do that! Sorry this is my first time doing this ^^;

Will get it done asap (today)

@GaijinKa
Copy link
Member

@suppresseddev Absolutely no problem, we’re happy to help you on your first ride if you need anything!

…inputted color formats, replaced additional usages of hardcoded color formats
@suppresseddev
Copy link
Author

Sorry for the delay, got super side tracked with university. Your recommendations have been applied.

I'm not entirely certain the ruff-linting worked? It made some changes with quotes, but I'm not sure if that is what you meant for me to do.

@b3by
Copy link
Collaborator

b3by commented Mar 3, 2026

Thank you again @suppresseddev for the changes in your PR! I still remember being flooded with uni stuff, so well done on that!

I reviewed your code, and there are still a number of things that need some attention. The most important one is that __slots__ cannot be used with class variables. This is on me I think, as I made some confusion with the implementation choices for the format class. Using __slots__ essentially prevents the creation of new fields for any given object, which speeds up performance, however, this does not work if slots include class variable like in the case of the PixelFormat class.

Thinking about it, having a class storing formats so that they can be uniformed across components does not really require constantly instantiating new objects. This leaves several options:

  • using a regular class (this is what you did)
  • using a dataclass that only has class attributes
  • using a string enum
  • using a named tuple with external utility functions (for conversion and such)
  • using a module

Deciding which option to go for is a matter of performance, usability and extensibility. On this regard, I tested some options to see where we stand in terms of access time and conversion time. I imagine these options could be implemented like this (I excluded the named tuple, as there is no real point in having that as it would still be absorbed by the module option):

from enum import StrEnum
from typing import ClassVar

from dataclasses import dataclass


# this is a bare class with only class variables
class FormatClass:
    @classmethod
    def normalize(cls, fmt: str) -> str:
        if not hasattr(cls, fmt.strip().upper()):
            return ""

        return fmt.strip().lower()

    RGB: ClassVar[str] = "rgb"
    BGR: ClassVar[str] = "bgr"
    ...


# this is a dataclass with class variables
@dataclass
class FormatDataclass:
    @classmethod
    def normalize(cls, fmt: str) -> str:
        if not hasattr(cls, fmt.strip().upper()):
            return ""

        return fmt.strip().lower()

    RGB: ClassVar[str] = "rgb"
    BGR: ClassVar[str] = "bgr"
    ...

# this is a string enum
class FormatEnum(StrEnum):
    @classmethod
    def normalize(cls, fmt: str) -> str:
        if not hasattr(cls, fmt.strip().upper()):
            return ""

        return fmt.strip().lower()

    RGB = "rgb"
    BGR = "bgr"
    ...

The module option might look like this:

# this is a separate format_module.py file
RGB = "rgb"
BGR = "bgr"
BGRA = "bgra"
YUV420P = "yuv420p"
YUV444P = "yuv444p"
YUV422 = "yuv422"
GRAY = "gray"
PAL8 = "pal8"


_formats = [RGB, BGR, BGRA, YUV420P, YUV444P, YUV422, GRAY, PAL8]


def normalize(fmt):
    if fmt.strip().upper() not in _formats:
        return ""

    return fmt.strip().lower()

I then measured the times:

import timeit


iterations = 2_000_000

print("class access")
print(timeit.timeit(lambda: FormatClass.RGB, number=iterations))

print("class conversion")
print(timeit.timeit(lambda: FormatClass.normalize("RGB"), number=iterations))

print()
print("dataclass access")
print(timeit.timeit(lambda: FormatDataclass.RGB, number=iterations))

print("dataclass conversion")
print(timeit.timeit(lambda: FormatDataclass.normalize("RGB"), number=iterations))

print()
print("enum access")
print(timeit.timeit(lambda: FormatEnum.RGB, number=iterations))

print("enum conversion")
print(timeit.timeit(lambda: FormatEnum.normalize("RGB"), number=iterations))

print()
print("module access")
print(timeit.timeit(lambda: format_module.RGB, number=iterations))

print("module conversion")
print(timeit.timeit(lambda: format_module.normalize("RGB"), number=iterations))

Execution times follow:

class access
0.043484051995619666
class conversion
0.3713115589998779

dataclass access
0.043302742997184396
dataclass conversion
0.37067307899997104

enum access
0.05861331799678737
enum conversion
0.38596753800084116

module access
0.04505628499464365
module conversion
0.2416251469985582

This seems to suggest that the module option would be the fastest. Using a module eliminates the need for a separate object holding formats and is quite portable. Of course, there is no safety on immutability, but the same is true for every other python module in existance! A small safeguard/reminder could be added defining those variables as final:

from typing import Final


RGB: Final[str] = 'rgb'
...

I would very much like to hear your opinion on this, as well as @GaijinKa 's, so that we can decide what is the best approach to take here.

As for the other issues with the PR, we have a number of automated checks in place that are executed whenever new code is submitted. One of these is the linting, a way of making sure every line of code respects a set of rules defined on the project. We use ruff, which you can install locally on your machine and run using the ruff.toml file as set of rules. When you do so, ruff will produce a list of all the known issues found. In the case of this PR, the automated linting detected the following:

  • in juturna/meta/_constants.py there is a missing newline at the end of the file (all files should end with an empty line)
  • in juturna/names/__init__.py there is an unused import (PixelFormat)
  • in juturna/names/_pixel_format.py line 6 is too long (in ruff the max line length we use is 80)
  • in plugins/nodes/proc/_yolo_detector/yolo_detector.py line 94 is too long

If you are considering keep up with contributing (which we really look forward to!), a great starting point is the contributing guidelines at the root of the project!

@suppresseddev
Copy link
Author

How do I use Ruff correctly with PyCharm, i have something running that did some auto fixes but it seems like it didn't encapsulate all of the changes you guys wanted.

@b3by
Copy link
Collaborator

b3by commented Mar 3, 2026

I am not 100% sure about PyCharm as I don't use it, but according to what I found online I believe they added native support for ruff at some point. If you are working with virtual environments, install ruff with pip within your environment, then enable it in PyCharm settings (make sure the binary path points at the ruff binary installed in the environment). After that you should be sorted. Alternatively, you can still use it from the command line with ruff check in the project root folder (of course, also in this case it should be installed in the current environment).

Hope this helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants