Skip to content

Convert Kiva constants to Python Enums #953

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants