Stackplot weighted_wiggle zero-area fix #6358

Merged
merged 3 commits into from May 16, 2016

Conversation

Projects
None yet
5 participants
Contributor

egpbos commented May 1, 2016

See #6313.

@egpbos egpbos Fixed stackplot when all lines are zero at some index.
506df7b

mdboom added the needs_review label May 1, 2016

tacaswell added this to the 2.0.1 (next bug fix release) milestone May 1, 2016

Owner

tacaswell commented May 1, 2016

Can you force-push just the first commit (with out the merge from master)?

Can you also add an image test that exercises this condition? It might be best to modify the test data in test_axes.test_stackplot_baseline and update the expected images.

Other wise 👍

Contributor

egpbos commented May 3, 2016

Force push done.

Owner

tacaswell commented May 6, 2016

Will you have time to add a test for this?

Contributor

egpbos commented May 8, 2016 edited

I'm trying, but I'm running on OSX in a Conda virtual environment, so having issues with that. The frameworkpython method doesn't seem to work for me, and I don't have that much time, unfortunately. Any ideas on getting my venv working?

Contributor

egpbos commented May 8, 2016

Just to be sure this makes sense, what I'd do is simply add

    d[50,:] = 0  # test for fixed weighted wiggle (issue #6313)

after line 1394 in matplotlib/tests/test_axis.py. To generate the baseline image, I have this script, but that's the one that won't run in my venv, so that's where I'm stuck.

Owner

efiring commented May 8, 2016

Others might have better methods, but here is what I do to develop mpl from within a conda environment on OSX:

  1. Use conda to make a test environment with the desired python version, and use conda to install matplotlib in it.
  2. Use conda to uninstall matplotlib.
  3. In my github clone of matplotlib, make a feature branch, and make the desired changes to it.
  4. In the root of that clone directory, 'pip install .'
  5. In a directory somewhere away from the cloned directory, to run your particular modified test, run:
    [path_to_matplotlib]/tests.py matplotlib.tests.test_axis.test_stackplot_baseline
  6. If everything worked as desired, copy the result images from the new result_images subdirectory of your testing directory to the appropriate baseline_images location in the repo, git commit everything, and push. If it didn't work, then modify the code, run 'pip uninstall matplotlib', and go back to 4.
Contributor

egpbos commented May 13, 2016

@efiring Awesome, that did the trick, thanks! Well, almost, I had to combine it with this pythonw/python.app tip. Perhaps an idea to put this method in the documentation? I assume steps 1 and 2 are to automatically install dependencies, right? Anyway, will try to generate the test images using this setup asap.

@egpbos egpbos Added updated image test for stackplot.
bb4bde2
Contributor

egpbos commented May 15, 2016

Ok, forgot to add matplotlib.style.use('classic') to the image generation script, that seems to fix the tests. By the way, is there a developers guide with info like this?

@egpbos egpbos Fixed new stackplot test images (classic style).
d1ac8f4
Contributor

egpbos commented May 16, 2016

@tacaswell: matplotlib.tests.test_mlab.rec2txt_testcase is failing in AppVeyor. Does that have anything to do with stackplot / this PR?

Owner

tacaswell commented May 16, 2016

No, that is a separate issue.

@tacaswell tacaswell merged commit 60731eb into matplotlib:master May 16, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 69.733%
Details

tacaswell removed the needs_review label May 16, 2016

@tacaswell tacaswell added a commit that referenced this pull request May 16, 2016

@tacaswell tacaswell Merge pull request #6358 from egpbos/stackplot_6313
FIX: Stackplot weighted_wiggle zero-area
102872a
Owner

tacaswell commented May 16, 2016

Thanks! I think this is your first contribution to the mpl source. Congratulations and hope we hear from you again.

Backported to v2.x as 102872a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment