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

remove some redunancy in _BaseAxes #2424

Closed
wants to merge 1 commit into from

Conversation

jarondl
Copy link

@jarondl jarondl commented Sep 16, 2013

While working on #1461, I've found that the __init__ of _BaseAxes
takes some redundant arguments. Of the 7 arguments it takes,
3 (xscale, yscale, label) can be removed without harm, because
they are automatically used in update(kwargs) (they have matching set_
methods). Another two have different names as setters: frameon == frame_on
and axisbg ~ axis_bgcolor. Note that the later are mispelled, as they
refer to axes and not axis. The following call is actually legal:

plt.subplot(111, axisbg='green', axis_bgcolor='red')

I do not know what to do with the other two arguments. (sharex sharey)

I've tried to remove this redunancy, keeping the old defaults
intact, and without breaking the api. My opinion is that axisbg and frameon
should be reomved, but api changes and deprectations are above my level.

See also issue #1461

While working on matplotlib#1460, I've found that the `__init__` of `_BaseAxes`
takes some redundant arguments. Of the 7 arguments it takes,
3 (`xscale, yscale, label`) can be removed without harm, because
they are automatically used in `update(kwargs)` (they have matching `set_`
methods). Another two have different names as setters: `frameon` == `frame_on`
and `axisbg` ~ `axis_bgcolor`. Note that the later are mispelled, as they
refer to axes and not axis. The following call is actually legal:

    plt.subplot(111, axisbg='green', axis_bgcolor='red')

I do not know what to do with the other two arguments. (`sharex` `sharey`)

I've tried to remove this redunancy, keeping the old defaults
intact, and without breaking the api. My opinion is that axisbg and frameon
should be reomved, but api changes and deprectations are above my level.

See also issue matplotlib#1460
@@ -412,7 +409,8 @@ def __init__(self, fig, rect,
#warnings.warn(
# 'shared axes: "adjustable" is being changed to "datalim"')
self._adjustable = 'datalim'
self.set_label(label)

kwargs.setdefault('label','')
Copy link
Member

Choose a reason for hiding this comment

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

PEP8: space after comma

@WeatherGod
Copy link
Member

I have some reservations about the changes here. While I definitely do think that we need a major cleanup of how we set properties, and to be much more consistent about it, this change does it via the self.update() approach which, by itself isn't bad, it does touch upon parts of the codebase that I have always detested.

This definitely need some further discussion, I think.

@jarondl
Copy link
Author

jarondl commented Sep 16, 2013

Well I definitely agree that this needs discussion.
All in all, my Python education has taught me to use kwargs only if there is no other option.
But, I think that the current situation is worse. I've shown an example above regarding axisbg and axis_bgcolor
Here is another weirdness:

gcf().add_axes([0.2,0.2,0.6,0.6], '',True, None, None, '', 'linear', xscale = 'log')

You can pass xscale as positional and as named argument.

On top of that, of the four methods that create axes, none have a correct
documentation of the parameters (#1461).

So in this PR, I tried to homogenise the parameters with as little intervention as possible.
It would be preferred to have a complete overhaul of the entire system. I hope
this is planned for some time in the future.

@mdboom
Copy link
Member

mdboom commented Sep 16, 2013

MEP13 contains one such attempt to redo this stuff, but I think there's also alternative proposals that would deserve a more thorough review.

@jarondl
Copy link
Author

jarondl commented Sep 17, 2013

I really like MEP13 and MEP10. What is their current status? Were they approved?
Can I contribute to, say MEP10? My searches of the mailing-list archive and the wiki
were inconclusive.

@WeatherGod
Copy link
Member

On Tue, Sep 17, 2013 at 1:42 AM, jarondl notifications@github.com wrote:

I really like MEP13 and MEP10. What is their current status? Were they
approved?
Can I contribute to, say MEP10? My searches of the mailing-list archive
and the wiki
were inconclusive.

MEP10 is a work-in-progress. A lot of effort went into getting 1.3 to
follow MEP10, and now it continues in the master branch. If you notice any
documentation that isn't conforming to MEP10 in the master branch, then
just simply make a PR against the master branch with updated documentation
(and add the PR number to the wiki page for MEP10, of course).

MEP13 is in a completely different state. There was much discussion, but no
consensus. I think a new MEP will have to be put forth soon with a much
narrower vision based upon discussions between Mike and I at SciPy 2013.

@jarondl jarondl closed this Sep 22, 2013
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

3 participants