Fix for marker verts bug

[lkilcher]

This is a proposed solution for #7807 . I added the checks for array ndim and shape, so that the ValueError('Unrecognized marker style...)' error is raised if an incorrectly-shaped np.ndarray is specified as input.

[codecov-io]

Current coverage is 62.14% (diff: 100%)

Merging #7809 into master will increase coverage by 0.04%

@@             master      #7809   diff @@
==========================================
  Files           174        175     +1   
  Lines         56050      56134    +84   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          34805      34883    +78   
- Misses        21245      21251     +6   
  Partials          0          0          

Powered by Codecov. Last update d7879bc...91191ce

[NelleV]

What does BF stand for?

[dopplershift]

Bugfix?

[lkilcher]

Yes. Bug fix.

[NelleV]

Can you add a regression test?

The patch looks fine to me elsewise.

[phobson]

Could you write a test that demonstrates that this fix works properly?

[NelleV]

@lkilcher I've create a pull request onto your branch with some tests and an additional fix.

[NelleV]

It looks good to me. 👍

Thanks @lkilcher !

[phobson]

Just a minor correction to your comments

[phobson]

checking that this does fail

[lkilcher]

Fixed. 23ce141

[lkilcher]

Thank you @NelleV , especially for the test!

Also, am I correct in my understanding of how Travis-CI (and other checks) work: they run the changes in the PR merged with HEAD of master (wherever that is at the time the PR is modified/updated)?

[NelleV]

No, travis-ci runs the test as they are on your branch. It doesn't merge into master, but once the pull request gets merged into master, it reruns the tests there.

Just a side note; HEAD is a tag that references the branch or the commit you are in. There's only one HEAD (so $HEAD of master" doesn't really mean anything in git jargon, but I think you are referring to matplotlib/matplotlib's master branch :) )

[lkilcher]

Thanks for the clarification. I suppose that's why the contributing doc suggests rebasing onto upstream master (is that the right jargon?) when finalizing a commit? Should I do that?

[tacaswell]

@NelleV I am pretty sure that travis runs against a commit which is the merge of the current branch into master (this is why fixing CI on master and re-starting PRs propagates the fix).

@lkilcher The rebase on master should only be if there are conflicts (in contrast to merging master into your branch). Can you point me at where in the docs suggest that you should rebase if there are no conflicts?

[NelleV]

@tacaswell really? I don't see the merge happening in the travis log

[lkilcher]

I think it was here, but now that I look at that it's fairly clear that this only needs to be done if there are conflicts. Perhaps it was an old version of that page, but more likely I'm just imagining things. Sorry!

[tacaswell]

The head it pulls from GH has the merge already done. The way the decide if the branch can be merged is by merging it and they expose that SHA out under as a specific ref. Copy the fetch logic from travis:

13:46 $ git fetch matplotlib +refs/pull/7976/merge
remote: Counting objects: 19, done.
remote: Compressing objects: 100% (19/19), done.
remote: Total 19 (delta 6), reused 0 (delta 0), pack-reused 0
Unpacking objects: 100% (19/19), done.
From git://github.com/matplotlib/matplotlib
 * branch                refs/pull/7976/merge -> FETCH_HEAD
(dd36) ✔ ~/source/matplotlib [fix_hatch_color L|✔] 
14:18 $ gitk
(dd36) ✔ ~/source/matplotlib [fix_hatch_color L|✔] 
14:18 $ gitk --all
(dd36) ✔ ~/source/matplotlib [fix_hatch_color L|✔] 
14:18 $ git checkout FETCH_HEAD
Note: checking out 'FETCH_HEAD'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at 2d8f488a8... Merge f6b4a32e6fb4d4687b40da3a81b0eefaad57a995 into 9b6f7c1aa36460648d9485caa61a258ebe852b78
(dd36) ✔ ~/source/matplotlib [:2d8f488a8|✔] 
14:19 $ git log --graph
*   commit 2d8f488a8ebaf2c3a27eef8968ad016f743c8400
|\  Merge: 9b6f7c1aa f6b4a32e6
| | Author: Thomas A Caswell <tcaswell@gmail.com>
| | Date:   Sun Jan 29 18:50:01 2017 +0000
| | 
| |     Merge f6b4a32e6fb4d4687b40da3a81b0eefaad57a995 into 9b6f7c1aa36460648d9485caa61a258ebe852b78
| | 
| * commit f6b4a32e6fb4d4687b40da3a81b0eefaad57a995
| | Author: Thomas A Caswell <tcaswell@gmail.com>
| | Date:   Sat Jan 28 18:27:08 2017 -0500
| | 
| |     FIX: Restore hatch color tracking edge color
| | 
* | commit 9b6f7c1aa36460648d9485caa61a258ebe852b78
|/  Author: Nelle Varoquaux <nelle.varoquaux@gmail.com>
|   Date:   Sun Jan 29 10:40:50 2017 -0800
|   
|       Merge pull request #7961 from jkseppan/issue7937
|       
|       Compute glyph widths similarly in Type 42 as in Type 3
| 
* commit 5a895daebb47b960a04be2a4619aa421b1839782
| Author: Nelle Varoquaux <nelle.varoquaux@gmail.com>
| Date:   Mon Jan 23 23:40:48 2017 -0800
| 
|     Merge pull request #7752 from newville/master
|     
|     bugfix for wx backend: release mouse on loss of focus and before trying to recapture
| 

This does leave some questions about exactly when that ref gets updated (and when the web-hooks get triggered).

[NelleV]

@lkilcher It seems that you'll have to rebase anyways… There is a conflict with master.

[lkilcher]

OK. I rebased and the tests passed, so I think this is good to go. :)

[dstansby]

Looks good, thanks @lkilcher

[dstansby]

Backported to v2.0.x via. 98a9347

[TAC edited to fix sha link]

[QuLogic]

The test shouldn't have been backported as is; it requires pytest which is not one of the dependencies of the 2.0.x line.