TST: splitlines in rec2txt test #6423

Merged
merged 4 commits into from May 17, 2016

Conversation

Projects
None yet
7 participants
Owner

tacaswell commented May 14, 2016

On windows the output has '\r\n' instead of '\n' for new
lines.

If this passes I plan to self-merge to un-break appveyor

@tacaswell tacaswell TST: splitlines in rec2txt test
On windows the output has '\r\n' instead of '\n' for new
lines.
966f50b
Owner

tacaswell commented May 14, 2016

attn @dopplershift I think this is an equivalent test .

Owner

tacaswell commented May 15, 2016

@janschulz Can you help out with the windows issue here? I do not have a system to test on and debugging via appveyor seems maddeningly painful!

Contributor

dopplershift commented May 15, 2016

👍 from me.

Owner

tacaswell commented May 16, 2016 edited

In windows this seems to output one less space is some cases:

windows = ['       x   y   s    s2', '   1.000   2   foo  bing ']
truth =   ['       x   y   s   s2',  '   1.000   2   foo bing ']
Owner

tacaswell commented May 16, 2016

Sorry, had order flipped, windows adds an extra space.

@tacaswell tacaswell FIX: compute str column width
The division was always returning 1, use length of first element
instead.
1900372
Owner

tacaswell commented May 16, 2016

@dopplershift I think there was a bug in computing the width of the string columns which for some reason was behaving differently on windows/linux.

Contributor

janschulz commented May 16, 2016 edited

I have a look... Looks like there are at least two problems:

The one where the link step can't find png.lib and the one where there is a space difference.

Re the png one: it seems that conda-forge recently added a libpng (and that one is installed), could be that there is a difference to the official one? https://github.com/conda-forge/libpng-feedstock/commits/master (cc: @ocefpaf)

Owner

tacaswell commented May 16, 2016

I think I have the space issue fixed

@dopplershift dopplershift and 2 others commented on an outdated diff May 16, 2016

lib/matplotlib/mlab.py
@@ -3188,9 +3188,7 @@ def get_justify(colname, column, precision):
ntype = column.dtype
if np.issubdtype(ntype, str) or np.issubdtype(ntype, bytes):
- # The division below handles unicode stored in array, which could
- # have 4 bytes per char
- length = max(len(colname), column.itemsize // column[0].itemsize)
+ length = max(len(colname), len(column[0]))
@dopplershift

dopplershift May 16, 2016 edited

Contributor

I'm not convinced len() will do what we want if the data for a column is ['foo', 'bars']. Probably should change the test to have some differing string sizes and see what happens.

@tacaswell

tacaswell May 16, 2016

Owner

is that allowed in recarrays? The types look like fixed-width string, not variable width.

@dopplershift

dopplershift May 16, 2016 edited

Contributor
In [18]: a = np.array([(1, 'foo'), (2, 'bars')], dtype=np.dtype([('i', np.int32), ('s', np.str, 4)]))

In [19]: a
Out[19]:
array([(1, 'foo'), (2, 'bars')],
      dtype=[('i', '<i4'), ('s', '<U4')])

In [20]: len(a['s'][0])
Out[20]: 3

I couldn't find a numpy function to get that 4 from the <U4, which is what we really want.

@dopplershift

dopplershift May 16, 2016

Contributor

But it turns out my original code was broken as well:

In [29]: a['s'][0].itemsize
Out[29]: 3

WTF?

@QuLogic

QuLogic May 17, 2016

Member

Maybe you want a.dtype['s'].itemsize == 16 ( / 4 for Unicode == 4)?

@tacaswell

tacaswell May 17, 2016

Owner

len() is not the right thing. The best I can come up with is int(a.dtype['s'].str[2:])

Owner

tacaswell commented May 17, 2016

And tests are passing, but it is dying as while trying to build the conda package

tacaswell added some commits May 17, 2016

@tacaswell tacaswell CI: disable conda package building
0cb2c3a
@tacaswell tacaswell FIX: try more reliable way to get string length
Fixed width byte/unicode dtypes seem to have the pattern
"[|<>][US][1-9][0-9]*" thus if we drop the first two charters we will
have the fixed width.
7765024
Owner

tacaswell commented May 17, 2016

ok, this is now passing (except for a down-load error on one of the appveyor tests).

I had to turn off the conda package building which is less than great, but makes the test useful again for code review.

@jenshnielsen jenshnielsen merged commit 326cc05 into matplotlib:master May 17, 2016

3 checks passed

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

mdboom removed the needs_review label May 17, 2016

jenshnielsen referenced this pull request May 17, 2016

Merged

Showraise gtk gtk3 #6417

tacaswell deleted the tacaswell:tst_fix_windows_print_test branch May 22, 2016

Contributor

cgohlke commented May 23, 2016

Is this going to be backported? matplotlib 1.5.2rc1 on Windows fails one test:

======================================================================
FAIL: test_csv2txt_basic (matplotlib.tests.test_mlab.rec2txt_testcase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "X:\Python35\lib\site-packages\matplotlib\tests\test_mlab.py", line 409, in test_csv2txt_basic
    assert_equal(mlab.rec2txt(a), truth)
AssertionError: '       x   y   s    s2\r\n   1.000   2   foo  bing \r\n   2.[16 chars]lah ' != '       x   y   s   s2\n   1.000   2   foo bing \n   2.000   3   bar blah '
-        x   y   s    s2
?                 -     -
+        x   y   s   s2
-    1.000   2   foo  bing
?                   -      -
+    1.000   2   foo bing
-    2.000   3   bar  blah ?                   -
+    2.000   3   bar blah

----------------------------------------------------------------------
Ran 5209 tests in 1399.371s
Owner

tacaswell commented May 23, 2016

Yes, sorry this got lost in the appveyor related issues.

On Sun, May 22, 2016, 20:12 Christoph Gohlke notifications@github.com
wrote:

Is this going to be backported? matplotlib 1.5.2rc1 on Windows fails one
test:

FAIL: test_csv2txt_basic (matplotlib.tests.test_mlab.rec2txt_testcase)

Traceback (most recent call last):
File "X:\Python35\lib\site-packages\matplotlib\tests\test_mlab.py", line 409, in test_csv2txt_basic
assert_equal(mlab.rec2txt(a), truth)
AssertionError: ' x y s s2\r\n 1.000 2 foo bing \r\n 2.[16 chars]lah ' != ' x y s s2\n 1.000 2 foo bing \n 2.000 3 bar blah '

  •    x   y   s    s2
    
    ? - -
  •    x   y   s   s2
    
  • 1.000 2 foo bing
    ? - -
  • 1.000 2 foo bing
  • 2.000 3 bar blah ? -
  • 2.000 3 bar blah

Ran 5209 tests in 1399.371s


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#6423 (comment)

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

@jenshnielsen @tacaswell jenshnielsen + tacaswell Merge pull request #6423 from tacaswell/tst_fix_windows_print_test
TST: splitlines in rec2txt test
cab8cf7
Owner

tacaswell commented May 23, 2016

backported to v1.5.x as cab8cf7

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