Fix incorrect text line spacing. #8495
Conversation
lib/matplotlib/tests/test_text.py
Outdated
| @@ -421,3 +421,34 @@ def test_font_scaling(): | |||
|
|
|||
| for i, fs in enumerate(range(4, 43, 2)): | |||
| ax.text(0.1, i*30, "{fs} pt font size".format(fs=fs), fontsize=fs) | |||
|
|
|||
|
|
|||
| def two_2line_texts(x1, y1, spacing1, x2, y2, spacing2): | |||
There was a problem hiding this comment.
You should parametrize this one test (it might have to wait for #8380, though.)
lib/matplotlib/tests/test_text.py
Outdated
| text_string = 'line1\nline2' | ||
| plt.text(x1, y1, text_string, linespacing=spacing1, alpha=0.5) | ||
| plt.text(x2, y2, text_string, linespacing=spacing2, alpha=0.5) | ||
| plt.show() |
lib/matplotlib/tests/test_text.py
Outdated
|
|
||
| def two_2line_texts(x1, y1, spacing1, x2, y2, spacing2): | ||
| text_string = 'line1\nline2' | ||
| plt.text(x1, y1, text_string, linespacing=spacing1, alpha=0.5) |
There was a problem hiding this comment.
It'd be preferable to use object-oriented API, I think.
lib/matplotlib/tests/test_text.py
Outdated
| @@ -421,3 +421,11 @@ def test_font_scaling(): | |||
|
|
|||
| for i, fs in enumerate(range(4, 43, 2)): | |||
| ax.text(0.1, i*30, "{fs} pt font size".format(fs=fs), fontsize=fs) | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize('spacing1, spacing2', [(2, 0.4), (0.4, 2)]) | |||
There was a problem hiding this comment.
I am very confused by what this is doing and how this relates to the added image.
There was a problem hiding this comment.
The bug with the text line spacing was that adding multiple text with different line spacing would not be correctly rendered. So this test would check that the intended line spacing for each the texts was applied. The second set of parameters was to check that calling text() in a different order would still produce the same result.
|
I am not sure that the image comparison plays niecly with paraterization (yet) https://codecov.io/gh/matplotlib/matplotlib/compare/1c0b96c592e2668f402cee131baeef9d1c2754bb...04ed906ab643afbc12028ab5bf846a6750dd28fe/diff looks like the body of the test is not being run. If you want to test the order like this in is probably better to put all 4 strtings on the same axes. |
|
Yes, would need #8380 probably. Is an image test strictly required? Could it check that the extent is different for the two test boxes? |
|
Using image test was just something I was familiar with, but I can update the tests to look at the extent instead. |
|
I also just merged #8380 so the paramterization should work now 😄. If you go the checking the exetents route, make sure to rebase the image out of the history and to make sure to force a draw ( |
|
@Keerush Are you comfortable doing a rebase? If so, see http://matplotlib.org/devel/gitwash/development_workflow.html?highlight=rebase#rebasing-a-pull-request-pr but you will want to do If not, I can take care of it. |
|
@tacaswell I haven't done a rebase before. I think it would be better if you did it. |
|
@Keerush You will need to do some fix up your master branch. The easiest way to do this is to simply delete your fork and local checkout, re-fork and re-clone. The other option is to do git checkout master
git remote matplotlib
git reset --hard matplotlib/master
git push --force origin masterAssuming that the remote name of your fork is 'origin' and the upstream repository is 'matplotlib' |
Added line spacing property to get_prop_tup() in text.py, so that the intended line spacing is applied to text. Added tests to check line spacing of text in plots are corrected rendered.
Fixes issue #7523.