neurosynth / ACE

Tools for automatic extraction of activation coordinates from published neuroimaging articles.
MIT License
43 stars 28 forks source link

FIX? test failures #1

Open dengemann opened 10 years ago

dengemann commented 10 years ago

@tyarkoni I'm trying to get tests up and running. This is what I get with python 2.7 (anaconda 1.8) on a recent MacBookPro (all dependencies listed in requirements.txt met from met via conda/pip).


======================================================================
ERROR: testFrontiersSource (ace.tests.test_ace.TestACE)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dengemann/github/ACE/ace/tests/test_ace.py", line 33, in testFrontiersSource
    article = source.parse_article(html)
  File "/Users/dengemann/github/ACE/ace/sources.py", line 362, in parse_article
    t = self.parse_table(table_html)
  File "/Users/dengemann/github/ACE/ace/sources.py", line 382, in parse_table
    return super(FrontiersSource, self).parse_table(table)
  File "/Users/dengemann/github/ACE/ace/sources.py", line 174, in parse_table
    return tableparser.parse_table(data)
  File "/Users/dengemann/github/ACE/ace/tableparser.py", line 215, in parse_table
    found_xyz = regex.search('\d+.*\d+.*\d+', '/'.join(r))  # use this later
TypeError: sequence item 17: expected string or Unicode, NoneType found

======================================================================
FAIL: testDatabaseProcessingStream (ace.tests.test_ace.TestACE)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dengemann/github/ACE/ace/tests/test_ace.py", line 71, in testDatabaseProcessingStream
    self.assertEqual(len(self.db.articles), 6)
AssertionError: 5 != 6
=====================================================================
FAIL: testPlosSource (ace.tests.test_ace.TestACE)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dengemann/github/ACE/ace/tests/test_ace.py", line 67, in testPlosSource
    self.assertEqual(t.n_activations, 24)
AssertionError: 10 != 24

======================================================================
FAIL: testScienceDirectSource (ace.tests.test_ace.TestACE)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/dengemann/github/ACE/ace/tests/test_ace.py", line 47, in testScienceDirectSource
    self.assertEqual(len(tables), 3)
AssertionError: 0 != 3

Any hint would be highly appreciated.

tyarkoni commented 10 years ago

@dengemann thanks for letting me know. The github repo may have been a bit out-of-date. I just pushed my local version, so try it again and let me know if you still have problems--all tests run fine on my machine.

tyarkoni commented 10 years ago

Gah, I just looked at the commit history and notice a "fixed tests" commit (b0843a35f63cdd594a7a8d204a5854c659e5d0e4) I forgot to push a few months ago. Sorry about that--hopefully that was indeed the problem. I'll close the issue, but please reopen if you still have problems.

dengemann commented 10 years ago

@tyarkoni thanks for you comments. I do have b0843a3 locally but still the tests don't pass. At least we got rid of the data processing stream failure ... I cannot re-open this issue unfortunately. I suspect there is an issue with different version of the many dependenices ...

tyarkoni commented 10 years ago

I wonder if this is a string encoding issue... it looks like the parsers are detecting some activations but missing others (e.g., the plos parser detects 10 rather than 24 activations). I've had to fight epic battles to get the parser to handle different publishers' character sets properly (mostly because I don't think I understand unicode very well), so it wouldn't surprise me terribly if there's still some glitch somewhere that's causing these issues. I'm also running 2.7, and just upgraded all my dependencies to the latest versions (still works fine for me), so I'm not sure the dependencies are the problem unless anaconda bundles an old version of one of them (maybe try upgrading with pip?).

Will dig into this later in the week and hopefully figure out what the issue is. Thanks again for flagging the problem!

dengemann commented 10 years ago

@tyarkoni thanks! I don't think the anaconda versions are terribly outdated, I've installed most of the dependencies via pip. Btw. it would be neat to replace the convoluted standard library unit-testing with a more stream-lined nose variant based on flat functions. This would make contributions + debugging of tests easier. However for this ideally the test suite should be running. Another thing that occurred to me is that licenses seem to be missing. I mention it here before opening related issues.

tyarkoni commented 10 years ago

In principle, I'd be happy to switch to nose, but I haven't used it before, so it's a matter of finding the time to learn it and then refactor the tests. Not sure it'll happen in the near term, but I'd certainly be happy to accept PRs along those lines.

There's a LICENSE file in the root folder... or are you referring to something else?

dengemann commented 10 years ago

Not sure it'll happen in the near term, but I'd certainly be happy to accept PRs along those lines.

cool. If you don't mind I'll open related issues + submit PRs in the future (I won't have the time to do it in the next days). Also pep8 and doc strings are areas potentially open to improvements. However it would be great to get the testing issue sorted out before. Setting up a Travis CI might help as well.

There's a LICENSE file in the root folder... or are you referring to something else?

Right. my bad. I'm used to license / author lines at the top of each file

I'll try to share a list of all my dependency versions such that we can rule out the trouble is related to this. Another thing I could do is take a closer look at the testing HTMLs. Probably I can fix it on my end.

WDYT?

tyarkoni commented 10 years ago

That all sounds great--much appreciated!

On Mon, Feb 3, 2014 at 1:42 PM, Denis A. Engemann notifications@github.comwrote:

Not sure it'll happen in the near term, but I'd certainly be happy to accept PRs along those lines.

cool. If you don't mind I'll open related issues + submit PRs in the future (I won't have the time to do it in the next days). Also pep8 and doc strings are areas potentially open to improvements. However it would be great to get the testing issue sorted out before. Setting up a Travis CI might help as well.

There's a LICENSE file in the root folder... or are you referring to something else?

Right. my bad. I'm used to license / author lines at the top of each file

I'll try to share a list of all my dependency versions such that we can rule out the trouble is related to this. Another thing I could do is take a closer look at the testing HTMLs. Probably I can fix it on my end.

WDYT?

Reply to this email directly or view it on GitHubhttps://github.com/neurosynth/ACE/issues/1#issuecomment-34003627 .