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

Convert Kiva constants to Python Enums #953

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

corranwebster
Copy link
Contributor

@corranwebster corranwebster commented Jul 16, 2022

This is largely backwards compatible, because we are aliasing to the original constant names, but it does remove constant names from kiva.__init__. People probably shouldn't have been using that instead of kiva.api, but this should probably be part of a major version bump.

@corranwebster corranwebster marked this pull request as ready for review July 16, 2022 19:04
Comment on lines +416 to +418
kiva_style |= constants.FontStyle.BOLD
if "italic" in style:
kiva_style += constants.ITALIC
kiva_style |= constants.FontStyle.ITALIC
Copy link
Member

Choose a reason for hiding this comment

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

We should perhaps note in the FontStyle declaration that it's used as a bitmask?

""" Sets the style for joining lines in a drawing.

Parameters
----------
style : join_style
The line joining style. The available
styles are JOIN_ROUND, JOIN_BEVEL, JOIN_MITER.
styles are Join.ROUND, Join.BEVEL, Join.MITER.
Copy link
Member

Choose a reason for hiding this comment

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

Should be LineJoin.* instead of Join.*

""" Specifies the style of endings to put on line ends.

Parameters
----------
style : cap_style
The line cap style to use. Available styles
are CAP_ROUND, CAP_BUTT, CAP_SQUARE.
are Cap.ROUND, Cap.BUTT, Cap.SQUARE.
Copy link
Member

Choose a reason for hiding this comment

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

Same here: LineCap.* instead of Cap.*

Comment on lines +1083 to +1085
if not (mode & DrawMode.STROKE):
self.state.line_state.line_color[3] = 0.0
if mode not in [FILL, EOF_FILL, FILL_STROKE, EOF_FILL_STROKE]:
if not (mode & (DrawMode.FILL | DrawMode.EOF_FILL)):
Copy link
Member

Choose a reason for hiding this comment

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

The word "flag" appears in the docstring for DrawMode, so I guess this is expected?

Perhaps the tension for me is that some of the constants are just enumerations, but others are bit positions in a mask?

Comment on lines +30 to +32
constants.LineJoin.ROUND: blend2d.StrokeJoin.JOIN_ROUND,
constants.LineJoin.BEVEL: blend2d.StrokeJoin.JOIN_BEVEL,
constants.LineJoin.MITER: blend2d.StrokeJoin.JOIN_MITER_BEVEL,
Copy link
Member

Choose a reason for hiding this comment

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

C++ enumeration naming is leaking on the Blend2D side 😆

Comment on lines -122 to +120
weight = WEIGHT_NORMAL
style = NORMAL
weight = FontWeight.NORMAL
style = FontStyle.NORMAL
Copy link
Member

Choose a reason for hiding this comment

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

A tangible improvement

Comment on lines 294 to 295
Note: this is a temporary method that will be removed in Enable 7.
"""
Copy link
Member

Choose a reason for hiding this comment

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

Is this the time to remove the code?

Comment on lines -17 to +18
BOLD, BOLD_ITALIC, DECORATIVE, DEFAULT, ITALIC, MODERN, NORMAL, ROMAN,
SCRIPT, TELETYPE, WEIGHT_BOLD, WEIGHT_LIGHT, WEIGHT_NORMAL, SWISS,
BOLD_ITALIC, FontFamily, FontStyle, FontWeight, DECORATIVE, DEFAULT,
MODERN, ROMAN, SCRIPT, TELETYPE, SWISS,
Copy link
Member

Choose a reason for hiding this comment

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

Is it too tedious to switch over to FontFamily.* throughout this file?

constants.BOLD_ITALIC: 'bold italic',
constants.FontStyle.NORMAL: 'regular',
constants.FontStyle.BOLD: 'bold',
constants.FontStyle.ITALIC: 'italic',
constants.FontStyle.BOLD | constants.FontStyle.ITALIC: 'bold italic',
Copy link
Member

Choose a reason for hiding this comment

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

Well... constants.BOLD_ITALIC does still exist.

Comment on lines -54 to +55
join = basecore2d.JOIN_MITER
cap = basecore2d.CAP_ROUND
join = constants.LineJoin.MITER
cap = constants.LineCap.ROUND
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for correcting this abuse of imports

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.

None yet

2 participants