add chemical datatypes #1941
add chemical datatypes #1941
Conversation
| <sniffer type="galaxy.datatypes.molecules:InChI"/> | ||
| <sniffer type="galaxy.datatypes.molecules:FPS"/> | ||
| <sniffer type="galaxy.datatypes.molecules:CML"/> | ||
| <!-- TODO: see molecules.py <sniffer type="galaxy.datatypes.molecules:SMILES"/>--> |
There was a problem hiding this comment.
Can you update the doctests for guess_ext with sniffer tests for these datatypes - at least the ones you have test data in the repo for:
https://github.com/galaxyproject/galaxy/blob/dev/lib/galaxy/datatypes/sniff.py#L259
There was a problem hiding this comment.
Actually, I already have some doctests. But I need to extend them and add more test data. Still struggling to find small once. Will do so!
There was a problem hiding this comment.
Your doctests are testing the sniff method - putting them up in guess_ext would test the actual datatype conf file - that the sniffer is in the right order, etc...
|
Conda is paying dividends all over the place, this is awesome @bgruening. |
bgruening
force-pushed
the
chemical_datatypes
branch
2 times, most recently
from
3e634c7 to
1187e79
Compare
|
@jmchilton can you please look over it. I'm not sure this all is needed. Why do I need to edit registry.py? |
|
@bgruening Does lib/galaxy/datatypes/test/2zbz.pdb have to have 4k lines? |
|
@martenson I can search for a smaller file. Do you know why I need to touch the registry.py? Shouldn’t be the ordering in datatypes_conf be sufficient? |
|
@bgruening I thought the conf is sufficient. Why do you think you need to touch registry.py? |
|
@martenson I need to have these changes to make tests pass: https://github.com/galaxyproject/galaxy/pull/1941/files#diff-81f0f003556ce7dde04d755ff6283680 |
|
@bgruening I cannot comment on what the best way is but I see no harm done by adding it there. |
bgruening
force-pushed
the
chemical_datatypes
branch
from
1187e79 to
c390efb
Compare
bgruening
force-pushed
the
chemical_datatypes
branch
from
c390efb to
1e9b0e6
Compare
|
@martenson I have found a smaller file and rebased this PR. |
|
The registry.py changes should not be needed, does this fail without them? |
|
@jmchilton I think that is what @bgruening said |
|
Okay - I see that now - I'll pull this down and hack on it. |
|
Clearly I hadn't made the change to test the datatypes conf that I thought I did - I've included the fix with bgruening#9. I wonder if this is the problem with #1156... |
Fix datatype sniff test to use sample and revert molecule registry ha…
|
Thanks @jmchilton! Seems to work as expected! Can we merge this and trigger a new test run in #1156. |
|
Thanks for the merge and the review and fixing and for everything! :) |
This will add a bunch of molecule datatypes to Galaxy. It was developed as part of the ChemicalToolBox and I feel it's time to move them into core.
The converters rely on a binary called OpenBabel which can will be installed during runtime, if conda dependency resolution is enabled.
I marked it as WIP as I have some smaller corrections to do, but would appreciate a first review and it it's ok to move them into main.