Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
MAINT: Updated tick and category test formatting#7993
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
lib/matplotlib/tests/test_ticker.py Outdated
| assert formatter.format_pct(x, display_range) == expected | ||
| class TestEndFormatter(object): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
typo: EngFormatter
| np.array(['1', '11', '3']), | ||
| [b'1', b'11', b'3'], | ||
| np.array([b'1', b'11', b'3'])], | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
apparently an extra ] here messing up travis?
😄 thanks! |
Thanks for looking through the PRs. I fixed up the issues you found. |
I am pretty sure that this PR is ready to go. The failing tests appear to be caused by an external source. |
6d2c93f to37592a7CompareNo more failing tests. |
f4b6f48 to37592a7Compare@dstansby. I've been seeing [MRG+1] a lot recently on PR titles. What does it mean? |
It means your PR has been approved by one reviewer (matplotlib asks for 2 approvals before commiting).http://matplotlib.org/devel/coding_guide.html#pr-review-guidelines |
Can you be the other one? |
lib/matplotlib/tests/test_ticker.py Outdated
| cases, especially the case when no SI prefix is present, for values in | ||
| [1, 1000). | ||
| Should not raise exceptions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
don't need this comment.
lib/matplotlib/tests/test_ticker.py Outdated
| Should not raise exceptions. | ||
| """ | ||
| unitless = mticker.EngFormatter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
these should probably be two separate tests (unitless and with unit) and possibly parameterized
Sorry thought I approved. Have two comments, but if you wanna punt those to a different improve engineering test PR, let me know and I'll merge. |
9d8d3ab toe3f721fCompare@story645 I made the improvements here. I kept is as a single parametrized test, removed the extra comment and also took care of some extra whitespace in ticker.py. |
85ee55b to5211dc9CompareNot an actual failure, Mac test just didn't run. |
With#7973 merged, this has conflicts. When you rebase, please be sure to leave out any empty |
I think on the rebase or something, you accidentally introduced a conflict. Please resolve. |
@QuLogic Should I replace |
Classic is the default now, it's not necessary if that's really what's being used (and I realize I forgot to remove a couple.) |
In that case I will remove those too. |
Just to make triple sure, I should make the replacement for |
That's correct. |
5211dc9 tof9f2cb1CompareDone. I ended up parametrizing a couple of the tests and adding one |
Again looks like Mac test didn't run. No actual failures. |
I've closed and reopened the PR to restart Travis and hopefully get the Mac test to run, but if you approve, this PR is ready to go. |
lib/matplotlib/tests/test_ticker.py Outdated
| 9.441, 12.588]) | ||
| assert_almost_equal(loc.tick_values(-7, 10), test_value) | ||
| def test_MultipleLocator_set_params(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seems like you forgot to remove the class in this one.
lib/matplotlib/tests/test_ticker.py Outdated
| fmt = ax.xaxis.get_major_formatter() | ||
| fmt.set_locs(ax.xaxis.get_majorticklocs()) | ||
| show_major_labels = [fmt(x) != '' for x in | ||
| ax.xaxis.get_majorticklocs()] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not aligned properly; better to wrap atfor.
lib/matplotlib/tests/test_ticker.py Outdated
| @pytest.mark.parametrize( | ||
| 'labelOnlyBase, exponent, locs, positions, expected', param_data) | ||
| @pytest.mark.parametrize('base', base_data) | ||
| def test_LogFormatterExponent(self, labelOnlyBase, base, exponent, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Class name is still here.
lib/matplotlib/tests/test_ticker.py Outdated
| class TestLogFormatterSciNotation(object): | ||
| testdata = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Whytestdata and nottest_data like the others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I wasn't sure if it would cause any problems if I started it withtest_.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We'll see how the tests go, then, but I'd guess this shouldn't be a problem since it isn't callable.
f9f2cb1 to3109ce9Compare@QuLogic Made the following changes:
|
Ready to go? |
Uh oh!
There was an error while loading.Please reload this page.
Rearranged tests in
lib/matplotlib/tests/test_ticker.pyand made a couple of very minor changes tolib/matplotlib/tests/test_category.py. Based on comment by@story645 in PRs#7482 and#7965.This PR should probably be merged before either of the others so I can rebase them onto it and clean things up.