Skip to content

DOC: MEP 28 Artist Refactor #4693

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

DOC: MEP 28 Artist Refactor #4693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df8a3a3
DOC MEP 28 Artist Refactor
OceanWolf Jul 14, 2015
06fa57b
DOC: Add to index
OceanWolf Jul 14, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions doc/devel/MEP/MEP28.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
=======================
MEP 28 Artist Refactor
=======================

.. contents::
:local:


Branches and Pull requests
==========================

All development branches containing work on this MEP should be linked to from here.

All pull requests submitted relating to this MEP should be linked to
from here. (A MEP does not need to be implemented in a single pull
request if it makes sense to implement it in discrete phases).

Abstract
========

During the course of MPL, the number and complexity of artists have grown, to become quite unmanageable, this PR seeks to address that by refactoring the Artists and creating a more sensible class structure.
Copy link
Member

Choose a reason for hiding this comment

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

I think all of our rst files should use hard returns to keep line lengths reasonable. I haven't checked, but I assume this has been done in the past.



Detailed description
====================

The goal of this MEP lies in automating and reducing as much of the "admin" code as possible making it easier to see what goes on and where and make faster changes to the codebase. A lot of the artist code for the example gets taken up with simple getter/setters, very laborious, and when it comes to the documentation it looks like a getter/setter desert. More so, we the same properties duplicated throughout the codebase, such as fillcolor, where we have ``set_fillcolor`` methods in ``lines``, ``markers`` and ``patches``. This makes readability and maintainability very difficult and just adds to the Artist doc desert.

With a clearer structure it also makes it easier to add new features such as keeping the legend upto date with its parent artists, add make it easier to implement dynamical property changing.


Implementation
==============

## Step 1 - Remove getter/setters
The easiest and biggest reduction comes from removing all of the ``artist.set_xxx(value)`` methods (where possible).
Copy link
Member

Choose a reason for hiding this comment

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

By using plain attributes, or by using properties? With the latter, we will still have getters and setters, but not as part of the API.

We will maintain a list of the properties the Artist class held as a class property.
To maintain BC we will add a single ``__getattr__`` in the Artist base class to return a closure to set a dict property.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what you mean here.


With this done, we can easily create property getter/setters, i.e. ``artist.xxx = value`` with ``__setattr__``.
Copy link
Member

Choose a reason for hiding this comment

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

It sounds like you are talking about packing a lot into getattr and setattr, but I probably won't understand until I see an example.

Create the getterHere we plan to use dictionary to manage the Artist properties.

The benefit of this, we can mark Artists as "stale" in a single place.


## Step 2 - Class inheritance
With a reduced codebase it becomes easier to assess how to split the classes.

To start with we shall use the following refactor rule:
If a keyword gets used by multiple artists that don't have direct ancestry, then it needs to go in a separate class. This prevents super keyword conflicts, but more importantly leads to good OOP design as it encourages full encapsulation of classes and a clearer API.

Once we have this cleaner structure we can then reassess this MEP to see what problems remain.


## Step 3 - Legend refactoring
As a natural consequence of the above steps, this will lead to a simplification of the legend. Using sets we can fine-tune which properties get updated.


Backward compatibility
======================

If we address the issue of alpha stacking in this PR then that would break BC, but we shall cross that bridge when we get to it.
1 change: 1 addition & 0 deletions doc/devel/MEP/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ Matplotlib Enhancement Proposals
MEP25
MEP26
MEP27
MEP28