search

cytoscape / py4cytoscape

Python library for calling Cytoscape Automation via CyREST
https://Py4Cytoscape.readthedocs.io
Other
69 stars 15 forks source link

Added ability to create a network view. #72

Closed nilsoberg2 closed 2 years ago

nilsoberg2 commented 2 years ago

Cytoscape doesn't automatically create views for large networks, and this code calls the REST API to create a view. The code as-is does not check if a view already exists; the user ought to be intelligent enough to do this themselves. See cytoscape/py4cytoscape#71 for an example of a large network.

bdemchak commented 2 years ago

Thanks, Nils ... this looks pretty reasonable. It would need to be merged into the 1.1.0 branch, which is the current working branch.

Before we do this, would you mind if I opened the discussion further so the RCy3 people can comment? We try to maintain function signature equivalence between the two APIs, and I'm sure they'll want to see this, too.

nilsoberg2 commented 2 years ago

Sorry - I wondered if I should have submitted the pull request on another branch.

Yes, please open the discussion to whoever you feel is relevant.

On Wed, Nov 17, 2021 at 11:37 AM Barry Demchak @.***> wrote:

Thanks, Nils ... this looks pretty reasonable. It would need to be merged into the 1.1.0 branch, which is the current working branch.

Before we do this, would you mind if I opened the discussion further so the RCy3 people can comment? We try to maintain function signature equivalence between the two APIs, and I'm sure they'll want to see this, too.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cytoscape/py4cytoscape/pull/72#issuecomment-971805330, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEEJXRILRVECN4ZOY2MLDWDUMPR4LANCNFSM5IHRCKZQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

bdemchak commented 2 years ago

Hi, Alex, Nils, Yihang and Kozo –

Nils has posted a push request for py4cytoscape, apparently addressing Cytoscape’s behavior for loading large networks. Recall that if a network is large enough, Cytoscape doesn’t create a view.

Nils’ PR adds create_view() to address this.

At first glance, this fills an obvious void, and I would think we’d be glad to have it.

Thinking further, there are two issues I can see …

  1. If py4cytoscape adopts it, it would be proper for RCy3 to adopt it, too … so we need the RCy3 perspective, too.

  2. In the back of my head, I seem to remember some sort of threshold (100,000 combined nodes and edges) for automatically applying filters (pre-3.9.0). I wonder if Cytoscape’s view creation threshold is related to the filter apply threshold. I wouldn’t think so, but it’s good to ask. And then ask whether there are other interactions we need to consider.

While I don’t argue the need for something like create_view(), I wonder if there’s also a case for setting a force_create parameter in functions like import_network_from_file(). We’d have to hit all of the network load functions if we went this route.

So, should we be having the parameter, the create_view() or both?

Comments??

From: Nils Oberg @.> Sent: Wednesday, November 17, 2021 12:17 PM To: cytoscape/py4cytoscape @.> Cc: Subscribed @.***> Subject: [cytoscape/py4cytoscape] Added ability to create a network view. (PR #72)

Cytoscape doesn't automatically create views for large networks, and this code calls the REST API to create a view. The code as-is does not check if a view already exists; the user ought to be intelligent enough to do this themselves. See #71 https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_cytoscape_py4cytoscape_issues_71&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=fFu6B65cWAJ7AgUFjDwKKTrHtVNCXj5eJdNbk9ApsGc&e= for an example of a large network.

You can view, comment on, or merge this pull request online at:

https://github.com/cytoscape/py4cytoscape/pull/72 https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_cytoscape_py4cytoscape_pull_72&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=cmJSAMVKbR5X4oP7_KAQyFCFmWwWA4kPCuDEdmXdagU&e=

Commit Summary

File Changes

(1 https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_cytoscape_py4cytoscape_pull_72_files&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=SxCa04zPNaF9RTNUg03C5Lb_56yb91sBVWJ6TSjPM4M&e= file)

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_cytoscape_py4cytoscape_pull_72&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=cmJSAMVKbR5X4oP7_KAQyFCFmWwWA4kPCuDEdmXdagU&e= , or unsubscribe https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AA4GLXQMDS7WU2UTD2PBIELUMPPQVANCNFSM5IHRCKZQ&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=gaNVMO694bqe8nEP5wnmYsooibtMWpy0UvVGW0_9Rhk&e= . Triage notifications on the go with GitHub Mobile for iOS https://urldefense.proofpoint.com/v2/url?u=https-3A__apps.apple.com_app_apple-2Dstore_id1477376905-3Fct-3Dnotification-2Demail-26mt-3D8-26pt-3D524675&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=QxJnANtEBd-6uSnjzKBsgfrZUfKXuf4cex9jLN9o2KM&e= or Android https://urldefense.proofpoint.com/v2/url?u=https-3A__play.google.com_store_apps_details-3Fid-3Dcom.github.android-26referrer-3Dutm-5Fcampaign-253Dnotification-2Demail-2526utm-5Fmedium-253Demail-2526utm-5Fsource-253Dgithub&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=ejgBFY6XRRCqr2d0PEvGGXQzRRR8G3mOE3-dKgxwqnY&e= . https://github.com/notifications/beacon/AA4GLXSKCGDXPZ7B73CTNHDUMPPQVA5CNFSM5IHRCKZ2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4PXXHYWA.gif

nilsoberg commented 2 years ago

Hi Barry.

Thanks for working with me on this issue.

Two comments:

  1. I almost prefer the force_create idea. create_view is more robust, but it almost seems more consistent with Cytoscape’s behavior to have a force_create parameter (meaning that Cytoscape automatically creates a view).

  2. I don’t think that this will address the issue I raised (#71) regarding the return value from import_network_from_file()

Thanks,

Nils

From: Barry Demchak @.> Sent: Wednesday, November 17, 2021 2:16 PM To: cytoscape/py4cytoscape @.> Cc: Oberg, Nils O @.>; Manual @.> Subject: Re: [cytoscape/py4cytoscape] Added ability to create a network view. (PR #72)

Hi, Alex, Nils, Yihang and Kozo –

Nils has posted a push request for py4cytoscape, apparently addressing Cytoscape’s behavior for loading large networks. Recall that if a network is large enough, Cytoscape doesn’t create a view.

Nils’ PR adds create_view() to address this.

At first glance, this fills an obvious void, and I would think we’d be glad to have it.

Thinking further, there are two issues I can see …

  1. If py4cytoscape adopts it, it would be proper for RCy3 to adopt it, too … so we need the RCy3 perspective, too.

  2. In the back of my head, I seem to remember some sort of threshold (100,000 combined nodes and edges) for automatically applying filters (pre-3.9.0). I wonder if Cytoscape’s view creation threshold is related to the filter apply threshold. I wouldn’t think so, but it’s good to ask. And then ask whether there are other interactions we need to consider.

While I don’t argue the need for something like create_view(), I wonder if there’s also a case for setting a force_create parameter in functions like import_network_from_file(). We’d have to hit all of the network load functions if we went this route.

So, should we be having the parameter, the create_view() or both?

Comments??

From: Nils Oberg @.> Sent: Wednesday, November 17, 2021 12:17 PM To: cytoscape/py4cytoscape @.> Cc: Subscribed @.***> Subject: [cytoscape/py4cytoscape] Added ability to create a network view. (PR #72)

Cytoscape doesn't automatically create views for large networks, and this code calls the REST API to create a view. The code as-is does not check if a view already exists; the user ought to be intelligent enough to do this themselves. See #71 <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_cytoscape_py4cytoscape_issues_71&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=fFu6B65cWAJ7AgUFjDwKKTrHtVNCXj5eJdNbk9ApsGc&e=%3E for an example of a large network.

You can view, comment on, or merge this pull request online at:

https://github.com/cytoscape/py4cytoscape/pull/72https://urldefense.com/v3/__https:/github.com/cytoscape/py4cytoscape/pull/72__;!!DZ3fjg!sAvLjiJqMi8I6BDRjXUi7hQbWjXWOcf8dsoe1FzaA_OEFdMEKcJPBx1e3fb6sALJnw$ <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_cytoscape_py4cytoscape_pull_72&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=cmJSAMVKbR5X4oP7_KAQyFCFmWwWA4kPCuDEdmXdagU&e=%3E

Commit Summary

File Changes

(1 <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_cytoscape_py4cytoscape_pull_72_files&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=SxCa04zPNaF9RTNUg03C5Lb_56yb91sBVWJ6TSjPM4M&e=%3E file)

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_cytoscape_py4cytoscape_pull_72&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=cmJSAMVKbR5X4oP7_KAQyFCFmWwWA4kPCuDEdmXdagU&e=%3E , or unsubscribe <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AA4GLXQMDS7WU2UTD2PBIELUMPPQVANCNFSM5IHRCKZQ&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=gaNVMO694bqe8nEP5wnmYsooibtMWpy0UvVGW0_9Rhk&e=%3E . Triage notifications on the go with GitHub Mobile for iOS <https://urldefense.proofpoint.com/v2/url?u=https-3A__apps.apple.com_app_apple-2Dstore_id1477376905-3Fct-3Dnotification-2Demail-26mt-3D8-26pt-3D524675&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=QxJnANtEBd-6uSnjzKBsgfrZUfKXuf4cex9jLN9o2KM&e=%3E or Android <https://urldefense.proofpoint.com/v2/url?u=https-3A__play.google.com_store_apps_details-3Fid-3Dcom.github.android-26referrer-3Dutm-5Fcampaign-253Dnotification-2Demail-2526utm-5Fmedium-253Demail-2526utm-5Fsource-253Dgithub&d=DwMCaQ&c=-35OiAkTchMrZOngvJPOeA&r=CsLWmDiVCCVSLXhL9WA-Kh4UUOkFEEy643k8U5VIUxE&m=WaWw501IY-re5Ie_P51u7UiENc8RRooNmqdj-AQxILT_B5voYlvo3Cenhg3ud0vS&s=ejgBFY6XRRCqr2d0PEvGGXQzRRR8G3mOE3-dKgxwqnY&e=%3E . https://github.com/notifications/beacon/AA4GLXSKCGDXPZ7B73CTNHDUMPPQVA5CNFSM5IHRCKZ2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4PXXHYWA.gifhttps://urldefense.com/v3/__https:/github.com/notifications/beacon/AA4GLXSKCGDXPZ7B73CTNHDUMPPQVA5CNFSM5IHRCKZ2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4PXXHYWA.gif*3E__;JQ!!DZ3fjg!sAvLjiJqMi8I6BDRjXUi7hQbWjXWOcf8dsoe1FzaA_OEFdMEKcJPBx1e3faPIGUA4Q$

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/cytoscape/py4cytoscape/pull/72*issuecomment-971946112__;Iw!!DZ3fjg!sAvLjiJqMi8I6BDRjXUi7hQbWjXWOcf8dsoe1FzaA_OEFdMEKcJPBx1e3fbo1bCSDA$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AI3P2JNEATUCDDXYAJBSHU3UMQEQDANCNFSM5IHRCKZQ__;!!DZ3fjg!sAvLjiJqMi8I6BDRjXUi7hQbWjXWOcf8dsoe1FzaA_OEFdMEKcJPBx1e3faYlldACg$. Triage notifications on the go with GitHub Mobile for iOShttps://urldefense.com/v3/__https:/apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675__;!!DZ3fjg!sAvLjiJqMi8I6BDRjXUi7hQbWjXWOcf8dsoe1FzaA_OEFdMEKcJPBx1e3faUlMHNMw$ or Androidhttps://urldefense.com/v3/__https:/play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign*3Dnotification-email*26utm_medium*3Demail*26utm_source*3Dgithub__;JSUlJSU!!DZ3fjg!sAvLjiJqMi8I6BDRjXUi7hQbWjXWOcf8dsoe1FzaA_OEFdMEKcJPBx1e3fZNawXzwA$.

bdemchak commented 2 years ago

Hi, Nils ...

By "#71", I think you mean that loading a large network throws an exception (though the network is actually loaded).

If so, I can say I wasn't present for the last developer meeting, though I'll be there for the next one (tomorrow). Looking at the meeting notes, it seems that the meeting was kept short due to the US holiday (Veterans Day ... Rememberance Day in parts of Europe?). I'll be sure to raise it tomorrow.

Is this what you're getting at, or is there a further connection to this #72?

nilsoberg2 commented 2 years ago

I misunderstood what you wrote. As you said, this PR exclusively address the issue that Cytoscape doesn't create a view for large networks. It is separate from cytoscape/py4cytoscape#71. Sorry for the misunderstanding.

AlexanderPico commented 2 years ago

In general, I see no problem with implementing functions reflecting existing CyREST and CyCommands. In particular, this one seems like a good addition.

bdemchak commented 2 years ago

Do you think you'll be implementing it in your near-imminent RCy3 release?

AlexanderPico commented 2 years ago

Ah, but it looks like the layout param is missing. See CyCommands docs for view create. We should go ahead and include all the supported options, right?

bdemchak commented 2 years ago

Yes. The PR is a great hint for functionality we're missing, but deserves a closer look to verify that it has all of the features we'd normally have in a function. If RCy3 implements this, then py4cytoscape will copy it.

AlexanderPico commented 2 years ago

Great. Here's how I'd do it in RCy3: https://github.com/cytoscape/RCy3/commit/e6570d312dbab373092c37b22f3c740aeeb5ee47

Note that I had to fix getNetworkViews and getNetworkViewSuid since they didn't work with NULLs previously. That may or may not be the case for py4cy.

Also, I clarified that the createView function only work if there is no pre-existing network view. Cytoscape still does not know what to do with multiple views per network though they are technically possible. So, we should avoid facilitating the creation of multiple views in automation, imo.

bdemchak commented 2 years ago

Looks clean to me ... The original PR had checks that we would normally make elsewhere, and I see that your code conserves them. I'll implement this tomorrow and add appropriate tests. Thanks.

bdemchak commented 2 years ago

Merged manually into 1.2 .... thanks, Nils and Alex!