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

Improve color provider #1055

Merged
merged 2 commits into from Nov 18, 2019
Merged

Improve color provider #1055

merged 2 commits into from Nov 18, 2019

Conversation

malefice
Copy link
Contributor

What does this changes

This will add a new color provider method color() that allows specifying a hue, a luminosity, and a color format to generate an matching color.

>>> from faker import Faker
>>> fake = Faker()

# Create random blue color in RGB format
>>> fake.color(hue='blue', color_format='rgb')
'rgb(5, 56, 175)'

# Create random bright red color in HSV format
>>> fake.color(hue='red', luminosity='bright', color_format='hsv')
'hsv(345, 96, 90)'

# Create random light pink color in HSL format
>>> fake.color(hue='pink', luminosity='light', color_format='hsl')
'hsl(294, 93, 71)'

# Create random color from yellow to green spectrum in hex format
>>> fake.color(hue=[50, 120], luminosity='dark', color_format='hex')
'#3f7c02'

What was wrong

There is currently no support for such features.

Notes

Personally, I prefer to return tuples of HSV, HSL, and RGB values instead of the color_format(X, Y, Z) string notation or any string for that matter, so that there is no need for users to parse the string for the values. I followed the current way rgb_color() and rgb_css_color() do this, but I am not a fan of those methods.

I also think that it would be better to provide more convenient methods and update old ones, so users will not need to remember the different color_format values while still being able to just specify 'blue' and 'dark' to get a color of that type.

Fixes #104

for color_name, color_attrs in self.colormap.items():
lower_bounds = color_attrs['lower_bounds']
s_min = lower_bounds[0][0]
s_max = lower_bounds[len(lower_bounds) - 1][0]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this equivalent to s_max = lower_bounds[-1][0]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I wonder why the author of randomcolor-py did not just use [-1] for this one. I will fix this, and I am sorry I did not notice.

if color['hue_range'][0] <= hue <= color['hue_range'][1]:
return self.colormap[color_name]
else:
raise ValueError('Value of hue is invalid.')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also show the value itself in the exception message? Something like 'Value of hue `%s` is invalid.' % hue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should only happen if something is wrong with COLOR_MAP itself, but sure.

v1 = int(v1)
v2 = int(v2)
except (ValueError, TypeError):
return [0, 360]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I'd want it to silently fallback to defaults. I'd rather have it throw an exception if the user is passing invalid values.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I will change this behavior.

@malefice
Copy link
Contributor Author

The flake8 errors are related to PR #1058.

@fcurella
Copy link
Collaborator

Looks Good! Thank you so much!

@fcurella fcurella merged commit bf75bea into joke2k:master Nov 18, 2019
@malefice malefice deleted the issue-104 branch December 4, 2019 09:22
@abtinmo abtinmo mentioned this pull request Dec 30, 2019
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.

Improve color provider
2 participants