style bypasses for node width and node height change current style to 'default'

[carissableker]

When using the style bypasses for node width and node height,

https://github.com/cytoscape/py4cytoscape/blob/e1466020ec7dbfb3cfdd438082a00976b3895d41/py4cytoscape/style_bypasses.py#L736

https://github.com/cytoscape/py4cytoscape/blob/e1466020ec7dbfb3cfdd438082a00976b3895d41/py4cytoscape/style_bypasses.py#L790

the message:

 style_name not specified, so updating "default" style.

is given, and the "active" style is changed to the default one (even when I am using a custom style).

The reason seems to be the call to lock_node_dimensions, which has the argument style_name, which defaults to 'default'.

https://github.com/cytoscape/py4cytoscape/blob/e1466020ec7dbfb3cfdd438082a00976b3895d41/py4cytoscape/style_dependencies.py#L160

A solution could be to use get_current_style(network) before calling lock_node_dimensions, since network is already an argument to the bypasses functions.

[bdemchak]

Hi, Carissa --

Thanks for catching this, and I apologize for this issue getting by us. My first thought would be to have all bypass functions accept a style parameter. Looking at the CyREST API, though, setting bypass values in a particular style isn't well supported.

Besides, your problem doesn't occur on bypass functions that don't call out to other style modules ... as you've pretty much pointed out.

So, that leads back to the solution you proposed ... these two functions do call out to other style modules, and therefore need to establish the intended style.

I'll work on this tomorrow ... the current development branch is 1.9.0, and that's where I'll put my changes. There are a few other bug fixes there ... see: https://github.com/cytoscape/py4cytoscape/blob/1.9.0/doc/release_log.rst

I'll let you know when the fix is in.

Thanks!

@yihangx @AlexanderPico

[yihangx]

Thanks Barry! I will sync in RCy3 when you finish in the 1.9.0 branch.

-Yihang

On Tue, Aug 8, 2023 at 6:41 PM Barry Demchak @.***> wrote:

Hi, Carissa --

Thanks for catching this, and I apologize for this issue getting by us. My first thought would be to have all bypass functions accept a style parameter. Looking at the CyREST API, though, setting bypass values in a particular style isn't well supported.

Besides, your problem doesn't occur on bypass functions that don't call out to other style modules ... as you've pretty much pointed out.

So, that leads back to the solution you proposed ... these two functions do call out to other style modules, and therefore need to establish the intended style.

I'll work on this tomorrow ... the current development branch is 1.9.0, and that's where I'll put my changes. There are a few other bug fixes there ... see: https://github.com/cytoscape/py4cytoscape/blob/1.9.0/doc/release_log.rst

I'll let you know when the fix is in.

Thanks!

@yihangx https://github.com/yihangx @AlexanderPico https://github.com/AlexanderPico

— Reply to this email directly, view it on GitHub https://github.com/cytoscape/py4cytoscape/issues/114#issuecomment-1670531658, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALDS53LB276FMP2UYTOMFILXULTEFANCNFSM6AAAAAA3IJPL4A . You are receiving this because you were mentioned.Message ID: @.***>

[bdemchak]

@yihangx @AlexanderPico

I have checked this change into 1.9.0 so that Carissa can try it. (Carissa ... please try it)

I'm pretty sure it works, but testing it is a different story. With the pre-fix code, set_node_width_bypass calls lock_node_dimensions, which sets Cytoscape's nodeSizeLocked attribute to False, I assume this is to allow the width to be set independent of the height. (Interestingly, nodeSizeLocked never gets returned to its prior state, but that's not my problem right now ... feel free to comment on this, though.)

The upshot of Carissa's complaint would be that the nodeSizeLocked attribute was getting set on the "default" style, not the current style (e.g., "galFiltered Style"). And py4cytoscape was spitting out a warning (like RCy3 would do).

So, Carissa's fix would solve this, and that's what I checked into 1.9.0.

But the unit tests were already working fine ... with or without Carissa's fix.

I can only conclude that the natural state of a style is to have nodeSizeLocked already False. It seems that the way to test correct operation of Carissa's fix would be to have nodeSizeLocked set to True on the current style so that node_lock_dimensions(False) would actually do something to avoid an error.

Can you tell me whether the above argument holds water ... and how to tell if nodeSizeLocked is set to cause trouble?

Pretty subtle, but I hate releasing something that's not clearly testable.

[bdemchak]

... or is the nodeSizeLocked lockout enforced only in the Cytoscape GUI and not at the CyREST level?

[carissableker]

Hi Barry,

Thanks for the fast fix and detailed discussion! In my case, I did not change the nodeSizeLocked setting, and in the GUI the "Lock node width and height" checkbox (I'm assuming the equivalent of nodeSizeLocked) was unchecked (but now I realise I am not sure to which style this refers, maybe in the default style it was checked at the time...).

Your fix works perfectly in my case. I also tested it with "Lock node width and height" checked, and it unchecked it quietly.

In the GUI, it seems the width and height bypasses and size lock play together rather confusingly. With "Lock node width and height" checked the width and height attributes are inaccesible.

But:

• Uncheck "Lock node width and height"
• Make an bypass in width and height for an individual node
• Check "Lock node width and height"
• The bypass is still applied

Maybe in the py4cytoscape bypass implementation, it would make sense, once the bypass is applied, to revert the nodeSizeLocked state to what it was before? I would find that more obvious, since I would not expect applying a bypass to a single node to change the style settings. I think bypasses are, at least meant to be, independent of the style. Indeed, changing the style preserves the bypass.

I think this is perhaps a more fundamental Cytoscape issue -- the node size lock should not affect the ability to apply width and height bypasses, as it is circumventable anyway.

Thanks again! For my use case, this is solved.

[bdemchak]

Excellent, Carissa ... thanks for your help!

I agree with your intuition regarding nodeSizeLocked, and my only issue with that is to explain why the existing acceptance tests were passing in the first place. These questions are for the Cytoscape crew to explore.

I'd give good odds that there could be further tweaks, so I'll leave this issue open so you see its progress.

Thanks for your patience and help!