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

Minor tick fix [backport to 1.4.x] #3612

Merged
merged 5 commits into from Dec 31, 2014
Merged

Conversation

nhmc
Copy link
Contributor

@nhmc nhmc commented Oct 3, 2014

This pull request ensures minor ticks are positioned correctly with AutoMinorLocator, fixing the bug described in issue #3557.

@tacaswell
Copy link
Member

Do the tests pass locally for you?

@tacaswell tacaswell added this to the v1.4.x milestone Oct 3, 2014
@tacaswell tacaswell changed the title Minor tick fix Minor tick fix [backport to 1.4.x] Oct 3, 2014
@nhmc
Copy link
Contributor Author

nhmc commented Oct 8, 2014

They do pass locally for me, although I might not be running them correctly.

I've fixed the pep8 errors (I hope). The other errors are with the tightlayout tests. I'm not sure why these are failing though, as they don't use minor ticks. I'd be grateful for any advice about how to fix this...

@tacaswell tacaswell modified the milestones: v1.4.x, v1.4.2 Oct 17, 2014
@tacaswell
Copy link
Member

@nhmc How are you running the tests?

The tests run from the installed version of mpl, not the source version.

@@ -36,6 +36,18 @@ def test_MultipleLocator():
assert_almost_equal(loc.tick_values(-7, 10), test_value)


def test_AutoMinorLocator():
Copy link
Member

Choose a reason for hiding this comment

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

The problem is that this test is creating a figure which is leaking out and getting re-used in the next test (global state strikes again!).

This test either needs an @cleanup decorator or to be re-written to test the locator without creating a figure/axes. I strongly prefer the second.

@tacaswell
Copy link
Member

expected:

tight_layout1-expected

Result:

tight_layout1

Note the limits match the new test.

@nhmc
Copy link
Contributor Author

nhmc commented Nov 14, 2014

Thanks for looking at this. I'll check in a fix soon.

On 14 November 2014 01:27, Thomas A Caswell notifications@github.com
wrote:

expected:

[image: tight_layout1-expected]
https://cloud.githubusercontent.com/assets/199813/5029677/22fd861e-6b17-11e4-8b29-1bf52f25bf31.png

Result:

[image: tight_layout1]
https://cloud.githubusercontent.com/assets/199813/5029686/2a958e76-6b17-11e4-9c24-81a581252626.png

Note the limits match the new test.


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

@tacaswell tacaswell merged commit c9b14f6 into matplotlib:master Dec 31, 2014
tacaswell added a commit that referenced this pull request Dec 31, 2014
BUG : fix minor tick placement

Merging by hand as I added a testing clean up commit on top of
the original branch.
tacaswell added a commit that referenced this pull request Dec 31, 2014
BUG : fix minor tick placement

Merging by hand as I added a testing clean up commit on top of
the original branch.
@tacaswell
Copy link
Member

cherry-picked as 50625de

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

2 participants