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

[Feature request] Add class Color #24

Open
Erol444 opened this issue Aug 16, 2022 · 4 comments
Open

[Feature request] Add class Color #24

Erol444 opened this issue Aug 16, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@Erol444
Copy link

Erol444 commented Aug 16, 2022

I would add a main class Color (naming TBD), and the whole library would work with it.

eg. distinct_color() returns Color. color_distance() takes 2x Color as argument. The why is that we don't need to create our own helper classes for normalization/color order conversion.

distinctipy.distinct_color().bgr().normalize(256).tuple() # Returns tuple of ints, 0..255, BGR order
# Same as:
distinctipy.distinct_color().cv2() # New proposed method


color = distinctipy.distinct_color()
cv2.putText(image, 'Text', (20,20), font, 1, color.cv2(), thickness, cv2.LINE_AA)
cv2.putText(image, 'Text', (20,40), font, 1, color.get_text_color().cv2(), thickness, cv2.LINE_AA)

Let me know your thoughts, as I (or one of our team members) would like to start working on this.
Thanks, Erik

@jack89roberts
Copy link
Collaborator

jack89roberts commented Aug 16, 2022

Hi @Erol444 , thanks for using distinctipy! I think I'd be happy for this to be added to distinctipy because I can see some of the benefits, but note that:

  • I'd like distinctipy to have as few dependencies as possible (but extra dependencies for optional features is ok)
  • I'd like any changes to be backwards compatible, as much as possible (if they're not we can discuss whether the change is worth a major version bump)

But PRs certainly welcome 🙂

@jack89roberts jack89roberts added the enhancement New feature or request label Aug 16, 2022
@Erol444
Copy link
Author

Erol444 commented Aug 16, 2022

Draft PR here: #25

Head up: for our project (depthai-sdk) we might fork this to remove dependencies to panda/matplotlib and create a separate library, as we don't require such features (only create unique colors).

@jack89roberts
Copy link
Collaborator

Draft PR here: #25

Great, I'll take a look when I have some time (hopefully later this week)

Head up: for our project (depthai-sdk) we might fork this to remove dependencies to panda/matplotlib and create a separate library, as we don't require such features (only create unique colors).

@Erol444 The latest versions of distinctipy only have numpy as a core dependency actually, the pandas/matplotlib/etc. dependencies are optional extras now, which you should only get if you do pip install 'distinctipy[optional]' or pip install 'distinctipy[all]'. If you're still getting them from pip install distinctipy that's a bug.

@Erol444
Copy link
Author

Erol444 commented Aug 18, 2022

I have now noticed I was on the old branch (develop) and that main has had a few updates. Now I see that matplotlib/pandas aren't dependencies anymore, which is perfect!
Here's the PR, you can link it to this feature request (github issue): #26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants