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

Fix for Python 3 in io.dpy #1100

Merged
merged 6 commits into from Jul 29, 2016

Conversation

Projects
None yet
5 participants
@ghoshbishakh
Member

ghoshbishakh commented Jul 28, 2016

No description provided.

@arokem

This comment has been minimized.

Member

arokem commented Jul 28, 2016

Thanks for doing this! Could you please write a test that fails without this fix?

@ghoshbishakh

This comment has been minimized.

Member

ghoshbishakh commented Jul 28, 2016

@arokem sure but can you guide me a bit where to write it. As this is a tutorial.

@coveralls

This comment has been minimized.

coveralls commented Jul 28, 2016

Coverage Status

Coverage remained the same at 82.908% when pulling da4834f on ghoshbishakh:python3fix_streamline_formats into c09a194 on nipy:master.

@codecov-io

This comment has been minimized.

codecov-io commented Jul 28, 2016

Current coverage is 80.84% (diff: 50.00%)

Merging #1100 into master will not change coverage

@@             master      #1100   diff @@
==========================================
  Files           200        200          
  Lines         23023      23023          
  Methods           0          0          
  Messages          0          0          
  Branches       2309       2309          
==========================================
  Hits          18614      18614          
  Misses         3933       3933          
  Partials        476        476          

Powered by Codecov. Last update c09a194...79bdffd

@arokem

This comment has been minimized.

@ghoshbishakh

This comment has been minimized.

Member

ghoshbishakh commented Jul 28, 2016

@arokem Actually without the changes in this PR, the test already fails:

bishakh@bisu-arch ~/projects/dipy/dipy/io (git)-[master] % python3 -m "nose"   
..E..
======================================================================
ERROR: dipy.io.tests.test_dpy.test_dpy
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/usr/lib/python3.5/site-packages/numpy/testing/decorators.py", line 147, in skipper_func
    return f(*args, **kwargs)
  File "/home/bishakh/projects/dipy/dipy/io/tests/test_dpy.py", line 24, in test_dpy
    dpw = Dpy(fname, 'w')
  File "/home/bishakh/projects/dipy/dipy/io/dpy.py", line 70, in __init__
    ['0.0.1'], 'Dpy Version Number')
  File "/usr/lib/python3.5/site-packages/tables/_past.py", line 35, in oldfunc
    return obj(*args, **kwargs)
  File "/usr/lib/python3.5/site-packages/tables/file.py", line 1152, in create_array
    obj=obj, title=title, byteorder=byteorder)
  File "/usr/lib/python3.5/site-packages/tables/array.py", line 188, in __init__
    byteorder, _log)
  File "/usr/lib/python3.5/site-packages/tables/leaf.py", line 262, in __init__
    super(Leaf, self).__init__(parentnode, name, _log)
  File "/usr/lib/python3.5/site-packages/tables/node.py", line 267, in __init__
    self._v_objectid = self._g_create()
  File "/usr/lib/python3.5/site-packages/tables/array.py", line 205, in _g_create
    raise TypeError("Array objects cannot currently deal with void, "
TypeError: Array objects cannot currently deal with void, unicode or object arrays

----------------------------------------------------------------------
Ran 5 tests in 0.016s

FAILED (errors=1)
Closing remaining open files:test.bin...done
@ghoshbishakh

This comment has been minimized.

Member

ghoshbishakh commented Jul 28, 2016

@arokem what version of pytables is travis using in the tests?

@ghoshbishakh

This comment has been minimized.

Member

ghoshbishakh commented Jul 28, 2016

@arokem So the problem is with the @iftables decorator. Travis is skipping this test as pytables is not installed in travis. But why it is done like this to skip a test deliberately when pyables is not installed?

@arokem

This comment has been minimized.

Member

arokem commented Jul 28, 2016

It used to be fairly complicated to install, so I think this is here so that people aren't obliged to install pytables.

Let's see if we can get it in: #1101

@ghoshbishakh

This comment has been minimized.

Member

ghoshbishakh commented Jul 28, 2016

@arokem so pr #1101 should be failing in tests. And then once that is merged this pr will be passing the tests right? Then let me know what should be my next steps when that is merged. This is quite interesting to see how TDD works 😃

@arokem

This comment has been minimized.

Member

arokem commented Jul 29, 2016

Exactly, once we sort that one out (actually #1102 now), you should be able
to rebase on master, and we should see whether this fix works.

On Thu, Jul 28, 2016 at 10:58 AM, Bishakh Ghosh notifications@github.com
wrote:

@arokem https://github.com/arokem so pr #1101
#1101 should be failing in tests. And
then once that is merged this pr will be passing the tests right? Then let
me know what should be my next steps when that is merged. This is quite
interesting to see how TDD works 😃


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1100 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAHPNrY5JNP8xOQAPb8Jj1i9OOsYtqSaks5qaO2ogaJpZM4JXWtm
.

TST: add pytables to travis-ci, Py35 full test run
Add a Python 3.5 run with full dependendencies.  Add (py)tables to full
dependencies.

@arokem arokem changed the title from Fix for Python3 in streamline_formats.py to Fix for Python 3 in io.dpy Jul 29, 2016

arokem and others added some commits Jul 29, 2016

BF: This raises a warning on line 367 otherwise.
And that triggers a test-failure.
Merge pull request #4 from arokem/shm-warning
BF: This raises a warning on line 367 otherwise.
@coveralls

This comment has been minimized.

coveralls commented Jul 29, 2016

Coverage Status

Coverage remained the same at 82.908% when pulling 1173135 on ghoshbishakh:python3fix_streamline_formats into c09a194 on nipy:master.

@coveralls

This comment has been minimized.

coveralls commented Jul 29, 2016

Coverage Status

Coverage remained the same at 82.908% when pulling 79bdffd on ghoshbishakh:python3fix_streamline_formats into c09a194 on nipy:master.

@arokem

This comment has been minimized.

@arokem arokem merged commit 63f6a5f into nipy:master Jul 29, 2016

3 of 4 checks passed

codecov/patch 50.00% of diff hit (target 80.84%)
Details
codecov/project 80.84% remains the same compared to c09a194
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 82.908%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment