ISA-tools / mzml2isa

Parser to get meta information from mzML file and parse relevant information to a ISA-Tab structure
GNU General Public License v3.0
12 stars 6 forks source link

Appveyor #11

Closed Tomnl closed 7 years ago

Tomnl commented 7 years ago

mzml2isa is now passing on Windows.

Martin (@althonos ), are you OK with this being merged?

Further details about changes in the closed issue

althonos commented 7 years ago

Hi @Tomnl, no objection about this, you did a great job with this PR !

Tomnl commented 7 years ago

Thanks @althonos. Before I merge, could you just check something please?

I have noticed something a bit strange whilst testing. There is an intermittent bug that only comes up when performing conversions using imzML files. This often causes the travis build to fail.

From my testing on my local computer it looks like this bug has been around before my changes.

Could you test on you machine with the original master branch code to see if you get the same error.

wget http://ftp.ebi.ac.uk/pub/databases/metabolights/studies/public/MTBLS273/BRB01C.ibd
wget http://ftp.ebi.ac.uk/pub/databases/metabolights/studies/public/MTBLS273/BRB01C.mzML
wget http://ftp.ebi.ac.uk/pub/databases/metabolights/studies/public/MTBLS273/BRB01C.imzML
wget http://ftp.ebi.ac.uk/pub/databases/metabolights/studies/public/MTBLS273/BRB01C.tif
wget http://ftp.ebi.ac.uk/pub/databases/metabolights/studies/public/MTBLS273/BRB01E.ibd
wget http://ftp.ebi.ac.uk/pub/databases/metabolights/studies/public/MTBLS273/BRB01E.imzML
wget http://ftp.ebi.ac.uk/pub/databases/metabolights/studies/public/MTBLS273/BRB01E.tif

mkdir imzML
mv BRB* imzML

python mzml2isa/mzml2isa/parsing.py -i imzML/ -o test/ -s test1

I then repeated python mzml2isa/mzml2isa/parsing.py -i imzml/ -o test/ -s test1 10 times and got an error 5 times ( I get a similar rate of failure for both the master and appveyor branch). The following error is received:

python mzml2isaOLD/mzml2isa/parsing.py -i test/imzml/ -o test/ -s test1
Traceback (most recent call last):                              | ETA:  --:--:--
  File "mzml2isaOLD/mzml2isa/parsing.py", line 305, in <module>
    run()
  File "mzml2isaOLD/mzml2isa/parsing.py", line 172, in run
    args.split, args.merge, args.verbose, args.jobs)
  File "mzml2isaOLD/mzml2isa/parsing.py", line 231, in full_parse
    metalist.append(parser(i, ont).meta)
  File "/usr/local/lib/python2.7/dist-packages/mzml2isa-0.4.28-py2.7.egg/mzml2isa/mzml.py", line 972, in __init__
    self.extract_meta(terms, xpaths_meta)
  File "/usr/local/lib/python2.7/dist-packages/mzml2isa-0.4.28-py2.7.egg/mzml2isa/mzml.py", line 237, in extract_meta
    self.cvParam_loop(elements, location_name, terms)
  File "/usr/local/lib/python2.7/dist-packages/mzml2isa-0.4.28-py2.7.egg/mzml2isa/mzml.py", line 256, in cvParam_loop
    self._descendents.update({k:self.obo[k].rchildren().id for k in terms[location_name] if not k in self._descendents})
  File "/usr/local/lib/python2.7/dist-packages/mzml2isa-0.4.28-py2.7.egg/mzml2isa/mzml.py", line 256, in <dictcomp>
    self._descendents.update({k:self.obo[k].rchildren().id for k in terms[location_name] if not k in self._descendents})
  File "/home/tnl495/.local/lib/python2.7/site-packages/pronto/ontology.py", line 233, in __getitem__
    return self.terms[item]
KeyError: 'IMS:1001212'

Can you think what could be causing this?

althonos commented 7 years ago

This is of course an ontology import issue, after an quick investigation I found the pronto obo parser is faulty here, I need to rewrite it anyway.

As a temp workaround you could check that 'IMS:1001212' in _ims when importing imagingMS.obo and if not then try reimporting it.

I'll patch pronto once i find some time to do it and the motivation to rewrite the parser (I think that will solve some multiprocessing issues too).

Tomnl commented 7 years ago

Thanks for the quick reply and investigation @althonos.

To get the PR to pass in travis I just rebuilt untill it passed but I will add a quick workaround patch like you described. I will do this after I merge the current branch though.

Thanks

althonos commented 7 years ago

Yeah, I used to do that too ^_^

I really need to update the parser, we keep getting so much issues because of it it's starting to be embarrassing for me :)

I pushed a commit in mzml2isa#appveyor implementing the patch, but it doesn't work 100%