Skip to content

Opened 140 html compliant colors - #4

Open
jdbodyfelt wants to merge 4 commits into
nabsabraham:masterfrom
jdbodyfelt:feature/full-html-css-colorlist
Open

Opened 140 html compliant colors#4
jdbodyfelt wants to merge 4 commits into
nabsabraham:masterfrom
jdbodyfelt:feature/full-html-css-colorlist

Conversation

@jdbodyfelt

Copy link
Copy Markdown

I added a JSON list found online of the 140 html-compliant (CSS3) color names, and modified the color protection to use this list within module.py. I tested it with two of my go-to colors (salmon & dodgerblue). Rendered fine at edotor.net.
OTHER COLORS SHOULD BE CHECKED!

@jdbodyfelt

Copy link
Copy Markdown
Author

Just noted a fix required on my L7 - recognizing local file on module import.

@nabsabraham

Copy link
Copy Markdown
Owner

thanks for this PR @jdbodyfelt! I don't have any CI set up, are you using this branch without issues? If so I will quickly review and merge

@jdbodyfelt

Copy link
Copy Markdown
Author

Hi! Maybe hold off on the merge - the file import of the namelist isn't working as a module yet. I'll either spend some time in figuring out how to path that right, or I'll just hard-code the namelist direct into module.py...

@jdbodyfelt

jdbodyfelt commented Mar 27, 2022

Copy link
Copy Markdown
Author

@nabsabraham - got around to a module-friendly fix using importlib.resources. I also added a graphviz render in the event that to_file() filename has a non-dot suffix.

Please check this branch yourself, and then feel free to merge!

@jdbodyfelt

Copy link
Copy Markdown
Author

@nabsabraham @ba-tno - I'm unable to add reviewers to this PR. If someone could pick that up and review? Thank you!

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is my first review, so not sure what is expected, hope my comments are helpful.

The core issue is that not all 140 colors are rendered by Edotor (click for example). I've marked them each with a review comment.
This is an Edotor issue, as both the Graphviz binary as well as another web-renderer (Magjac Graphviz Visual Editor) show them without any problem.

You could either remove the non-working colors, or suggest the Magjac renderer instead of Edotor. The Magjac renderer is also referred to in the Graphviz docs via the "Edit in Playground" links.

Comment thread pandaserd/colors.json
"families": ["red"]
},
{
"name": "DARKRED",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't work in Edotor

Comment thread pandaserd/colors.json
"families": ["purple", "orchid"]
},
{
"name": "FUCHSIA",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't work in Edotor

Comment thread pandaserd/colors.json
"families": ["purple", "medium"]
},
{
"name": "REBECCAPURPLE",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't work in Edotor

Comment thread pandaserd/colors.json
"families": ["purple", "dark", "orchid"]
},
{
"name": "DARKMAGENTA",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't work in Edotor

Comment thread pandaserd/colors.json
"families": ["green"]
},
{
"name": "LIME",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't work in Edotor

Comment thread pandaserd/colors.json
"families": ["gray"]
},
{
"name": "DARKGRAY",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't work in Edotor

Comment thread pandaserd/module.py
@@ -27,7 +34,9 @@ def __init__(self, table, table_name, **kwargs):
self.align = 'left'
self.font_color = 'grey60'
self.bg_color = kwargs.get('bg_color', 'grey')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The default color grey (British spelling) is not part of the self.html_colors list, gray (US spelling) is.

Comment thread pandaserd/module.py
self.bg_color = kwargs.get('bg_color', 'grey')
bg_colors = ['lightblue', 'skyblue', 'pink', 'lightyellow', 'grey', 'gold']
bg_colors = self.html_colors #['lightblue', 'skyblue', 'pink', 'lightyellow', 'grey', 'gold']
# TODO: Set font_color complementary to bg_color

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would be nice indeed! Two methods I can think if (perhaps you already have an idea how to do this):

  1. you can find the RGB of a complementary color from using this method and then find the closest available color in the colors.json file.
  2. you can create a dictionary once that lists the complementary color for each key in colors.json and do a lookup there.

Comment thread pandaserd/module.py
src.render(path.with_suffix(''), format=fmt)
print(f'image written to {filename}')
else:
url='https://edotor.net/'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perhaps add http://magjac.com/graphviz-visual-editor/ as an option as well? Given that Edotor does not render all the HTML colors from the list.

Comment thread setup.py
install_requires = [
'pandas'
]
python_requires=">=3.6",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure how this works, but given that pandas requires python >=3.8 already, perhaps best to write that here as well? Or does this refer to the python version this "bare" package (without dependencies) would require?

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.

2 participants