Skip to content

Normalize some signatures of Kiva methods#691

Merged
jwiggins merged 6 commits into
masterfrom
refactor/backend-normalization
Mar 9, 2021
Merged

Normalize some signatures of Kiva methods#691
jwiggins merged 6 commits into
masterfrom
refactor/backend-normalization

Conversation

@jwiggins
Copy link
Copy Markdown
Member

@jwiggins jwiggins commented Mar 8, 2021

This is part of an ongoing effort to address #648

  • Changes to method signatures were made in AbstractGraphicsContext to reflect how a majority of backends have something implemented (gradients, show_text, draw_path_at_points, set_line_dash).
  • show_text with the point= kwarg was refactored for many of the lesser-used backends.
  • select_font now has the correct signature on celiagg, PDF, and Quartz backends.
  • QPainter uses the fill color for text now, matching the behavior of AGG

Copy link
Copy Markdown
Contributor

@aaronayres35 aaronayres35 left a comment

Choose a reason for hiding this comment

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

LGTM

are there any documentation updates that need to happen? It looks like we use automethod now so I don't think there should.
Looking at that though I noticed we still have some old doc files in docs/kiva/agg. We can probably delete these? eg https://github.com/enthought/enable/blob/master/docs/kiva/agg/interface.txt

Comment thread kiva/qpainter.py
with self:
# Kiva uses the fill color for text
brush = self.gc.brush()
self.gc.setPen(brush.color())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

verified this with the benchmark 👍

Comment thread kiva/pdf.py
Comment on lines -650 to -662
def set_character_spacing(self):
"""
"""
pass

def get_character_spacing(self):
""" Get the current font """
raise NotImplementedError

def set_text_drawing_mode(self):
"""
"""
pass
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just to clarify, why did these get removed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They're covered by GraphicsContextBase

@jwiggins
Copy link
Copy Markdown
Member Author

jwiggins commented Mar 9, 2021

Documentation is covered by :automethod:. Basically, these changes are bringing the backend implementations in line with what's specified by that documentation.

@jwiggins jwiggins merged commit fe462b8 into master Mar 9, 2021
@jwiggins jwiggins deleted the refactor/backend-normalization branch March 9, 2021 08:51
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.

3 participants