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

BUG : raise exception in subplot if num out of range #3167

Merged
merged 3 commits into from Jul 11, 2014

Conversation

tacaswell
Copy link
Member

if subplot(rows, cols, num) is called with num not in
0 < num <= rows * cols raise a ValueError

close #3166

@tacaswell tacaswell added this to the v1.4.0 milestone Jun 29, 2014
@twiecki
Copy link

twiecki commented Jun 29, 2014

Cool, thanks for being so responsive!

@pelson
Copy link
Member

pelson commented Jun 30, 2014

@tacaswell - this has the potential to break a whole lot of code.
Firstly, we would need a really prominent api_changes entry, and secondly I'd like to get some feedback from some of the other devs (including @efiring, @WeatherGod and @mdboom).

Before, the following code would "work" by putting an axes on the bottom right (not the top left as one might expect):

plt.subplot(2, 2, 0)

The fix is sound, and the existing behaviour is undesirable, but it is potentially problematic for future version adoption...

@tacaswell
Copy link
Member Author

@pelson Some how I missed that consequence when I wrote this.

I will change this to raise a warning for 0 and claim we will turn it into an exception in 1.5

@WeatherGod
Copy link
Member

Devil's Advocate: why should negative values be bad? we do negative
indexing all the time in numpy.

Answering my own question... because negative indices make no sense in
1-base indexing?

On Mon, Jun 30, 2014 at 11:16 AM, Thomas A Caswell <notifications@github.com

wrote:

@pelson https://github.com/pelson Some how I missed that consequence
when I wrote this.

I will change this to raise a warning for 0 and claim we will turn it into
an exception in 1.5


Reply to this email directly or view it on GitHub
#3167 (comment)
.

@tacaswell
Copy link
Member Author

@pelson updated

@efiring
Copy link
Member

efiring commented Jul 1, 2014

Good move. I would not expect that even the original version would break code, given that the indexing in subplot has always followed the original Matlab convention, so zero has never been a valid value; but there certainly is no harm in using the intermediate deprecation.

@@ -177,6 +177,18 @@ original location:
treated as numpy fancy indexing and only the two markers corresponding to the
given indexes will be shown.

* The use of the index 0 has in `plt.subplot` and related commands is
Copy link
Member

Choose a reason for hiding this comment

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

grammar check please... I think you were trying for "as in", not "has in".

if subplot(rows, cols, num) is called with num not in
0 < num <= rows * cols raise a ValueError

close matplotlib#3166
@tacaswell
Copy link
Member Author

fixed and rebased

position. This is due to the indexing in subplot being 1-based (to
mirror MATLAB) so before indexing into the `GridSpec` object used to
determine where the axes should go, 1 is subtracted off. Passing in
0 results is passing -1 to `GridSpec` which results in getting the
Copy link
Member

Choose a reason for hiding this comment

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

0 results *in* passing...

@tacaswell
Copy link
Member Author

fixed and amended.

dopplershift added a commit that referenced this pull request Jul 11, 2014
BUG : raise exception in subplot if num out of range
@dopplershift dopplershift merged commit 5f5599b into matplotlib:master Jul 11, 2014
@tacaswell tacaswell deleted the subplot_index_fix branch August 28, 2014 03:02
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.

subplot(x, x, 0) should raise Exception
8 participants