Committing organism specification on list upload functionality

[nkengawoh]

@julie-sullivan

Fixes: intermine/intermine-ws-python-docs#33

Here is my update on this issue. Please could you review and give me feedback?

[pep8speaks]

Hello @nkengawoh! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file intermine/lists/listmanager.py:

Line 99:13: E741 ambiguous variable name 'l'
Line 101:21: E128 continuation line under-indented for visual indent
Line 189:9: E125 continuation line with same indent as next logical line
Line 212:9: E125 continuation line with same indent as next logical line
Line 282:25: E128 continuation line under-indented for visual indent
Line 301:30: E124 closing bracket does not match visual indentation
Line 309:33: E128 continuation line under-indented for visual indent
Line 316:37: E128 continuation line under-indented for visual indent
Line 439:9: E125 continuation line with same indent as next logical line
Line 460:9: E125 continuation line with same indent as next logical line
Line 481:9: E125 continuation line with same indent as next logical line
Line 502:9: E125 continuation line with same indent as next logical line
Line 524:9: E125 continuation line with same indent as next logical line
Line 558:9: E125 continuation line with same indent as next logical line

Comment last updated at 2019-03-09 13:17:04 UTC

[julie-sullivan]

1. Can you fix the errors first
2. What sorts of things do you think I should test? Do you have suggestions for what might "go wrong"?

My list so far:

• can make a list with an organism name
• can make a list without an organism name
• organism name -- doesn't exist, is misspelled etc. Is error handled properly?
• Can create a list of ontology terms. (these don't have an organism relationship)

Thanks!

[nkengawoh]

@julie-sullivan
I have corrected the PEP issues.
I do not have any more suggestions on your list of test cases.

Thanks!

[julie-sullivan]

@rachellyne Can you test this to make sure it does what you want?

[rachellyne]

@nkengawoh Can you check this with genes where there is the potential to create the wrong gene due to multiple entries with the same symbol? You can do this in humanMine with example genes pparg and pax6. If you query humanMine with either of these genes you will see there is an entry for human, mouse and rat. If we are uploading a list with this symbol, we probably only want the gene from a specified organism (say human). I can't tell whether this works from your example as there is only one gene for each in flymine, so it would automatically create the correct gene anyway.

[nkengawoh]

@rachellyne Thank you for pointing that out.
Here is the test you requested:

[nkengawoh]

@julie-sullivan @rachellyne
Have you had a look?

[yochannah]

@nkengawoh I'm taking a look at this now. Could you try merging the current intermine / dev branch into this PR? There don't seem to be any conflicts and it might fix the travis build failure! 👍

[yochannah]

My tests so far:

• can make a gene list with ONE organism name as a string
• can make a gene list with a list of MANY organism names.

In both cases above, any symbols for other organisms are simply dropped out from the resulting list.

• can make a list without an organism name like before both via query and via list of identifiers.
• organism name -- doesn't exist, is misspelled etc. Is error handled properly?
• Can create a list of ontology terms. (these don't have an organism relationship)

---

I tried mis-spelling an organism name and got back a 400 error. It might be nice to handle this more smoothly.

code:

## setting up
from intermine.webservice import Service

service=Service("www.humanmine.org/humanmine/service",token="8146s9z5Bel8Vfz7scX0")
lm = service.list_manager()

## These symbols exist for humans and for other organisms too. 
symbols= ["PPARG","PAX6"]

lm.create_list(content=symbols,
               list_type="Gene",
               name="my list with just humans", 
               organism=["Homo sapiedfgns"]) # this organism doesn't exist ;) 

Error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/intermine/results.py", line 640, in open
    return urlopen(req)
  File "/usr/lib/python3.5/urllib/request.py", line 163, in urlopen
    return opener.open(url, data, timeout)
  File "/usr/lib/python3.5/urllib/request.py", line 472, in open
    response = meth(req, response)
  File "/usr/lib/python3.5/urllib/request.py", line 582, in http_response
    'http', request, response, code, msg, hdrs)
  File "/usr/lib/python3.5/urllib/request.py", line 510, in error
    return self._call_chain(*args)
  File "/usr/lib/python3.5/urllib/request.py", line 444, in _call_chain
    result = func(*args)
  File "/usr/lib/python3.5/urllib/request.py", line 590, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 400: 400

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.5/dist-packages/intermine/lists/listmanager.py", line 243, in create_list
    data = self.service.opener.post_plain_text(uri, ids)
  File "/usr/local/lib/python3.5/dist-packages/intermine/results.py", line 622, in post_plain_text
    return self.post_content(url, body, InterMineURLOpener.PLAIN_TEXT)
  File "/usr/local/lib/python3.5/dist-packages/intermine/results.py", line 627, in post_content
    with closing(self.open(url, body, {'Content-Type': content_type})) as f:
  File "/usr/local/lib/python3.5/dist-packages/intermine/results.py", line 652, in open
    handler(*args)
  File "/usr/local/lib/python3.5/dist-packages/intermine/results.py", line 697, in http_error_400
    raise WebserviceError("There was a problem with our request", errcode, errmsg, message)
intermine.errors.WebserviceError: [Errno There was a problem with our request] 400: '400'

and my final note: We should probaby add unit tests for this scenario to

• https://github.com/intermine/intermine-ws-python/blob/dev/tests/test_lists.py
• https://github.com/intermine/intermine-ws-python/blob/dev/tests/live_lists.py

@julie-sullivan are you familiar enough with the employee test model to help find a comparable example without too much effort? If not I'm happy to look.

---

@rachellyne - you can test it by looking at tutorial 9 and humanmine_list_organism_constraints.ipynb here:

I did check my token in like a doofus so I've invalidated those tokens - you'll have to put your own in.

---

[nkengawoh]

@yochannah
Thanks for having a close look at this.
I have merged the current dev branch into this PR, and now the travis build passes.
You are right about the Error handling. I did not work on that.
I shall now handle errors concerning this PR in a better way; and shall also write unit tests for this PR.

For the sake of this PR/functionality, I think it is best that, after this, I create another issue for error handling in the repo, to handle such errors more generally.

[yochannah]

@nkengawoh let us know when you're ready for this to be re-reviewed! 😸

[nkengawoh]

@yochannah @rachellyne
Sorry for the silence and delay. I got caught up with a lot of things.

I have written the tests for this feature, however, I used flymine and not the employee test model; because I am not familiar with it and it does not have Genes, among other things.

However, I cannot fully run the live tests, they fail with the error:
raise ModelError("'" + name + "' is not a class in this model") intermine.model.ModelError: "'Employee' is not a class in this model"

Is there anything I missed?

[yochannah]

hmmm, testing for organism doesn't seem easy to translate to the employee model, does it.

@julie-sullivan - any suggestions?

[julie-sullivan]

no, no suggestions.

https://github.com/intermine/intermine-ws-python/tree/dev/tests - the bio tests seem to test on the live webapp.

Or make the code generic enough to be tested in the testmine. I know in the code we have abstracted it away to be extraValue, I don't know if that was also done in the testmine.

[nkengawoh]

@yochannah @julie-sullivan

Yes, it does not translate well to the employee model.

I think I will go with Julie's hint, but I shall complete this after I finish up the GSoC proposal, if that is fine by you.

Thanks.

[yochannah]

That's a good plan! :)

…

On Mon, 1 Apr 2019 at 14:16, nkengawoh ***@***.***> wrote: @yochannah <https://github.com/yochannah> @julie-sullivan <https://github.com/julie-sullivan> Yes, it does not translate well to the employee model. I think I will go with Julie's hint, but I shall complete this after I finish up the GSoC proposal, if that is fine by you. Thanks. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AI14jr2ue9bCcgafwrMRv2qlOMAVlwAHks5vcgbFgaJpZM4bOfXw> .

[julie-sullivan]

@yochannah can I merge this? We had another request for this functionality today! :)