search

PathwayCommons / chibe

Pathway visualization and analysis tool for Pathway Commons and other BioPAX data
Eclipse Public License 1.0
13 stars 10 forks source link

Compound node members could be sorted alphabetically #69

Closed ozgunbabur closed 6 years ago

ozgunbabur commented 6 years ago

ChiBE tiles the members of a Complex in mechanistic view, similarly when rendering a SIF network, ChiBE groups the nodes with the same topology, which helps for complexity management. However, when the compound node has too many members, it becomes difficult to find a member because they are not sorted alphabetically.

The current design of the tiling algorithm does not allow for sorting members because the tiling is not performed first left-to-right then up-to-bottom. Current algorithm always try to maintain a square while the compound grows with new members, hence it grows from topleft-to-bottomright. We may consider changing this design to favor member sorting.

ozgunbabur commented 6 years ago

The relevant part of the code to change this is below.

https://github.com/PathwayCommons/chibe/blob/master/src/main/java/org/gvt/layout/BiPaLayout.java#L173

ozgunbabur commented 6 years ago

There is some bug in the implementation. Try loading below sif (remove the .txt extension, github does not let me load .sif directly).

CREB1-neighborhood.sif.txt

You will see that compound members are mostly ordered alphabetically. Mostly! Look at the end of them.

metincansiper commented 6 years ago

@ozgunbabur getWith() in https://github.com/PathwayCommons/chibe/blob/master/src/main/java/org/gvt/layout/BiPaLayout.java#L287 is calling shiftToLastRow(). It seems that this call is the reason of the bug. Shall I remove that call and the method itself as well if it has no other usage?

ozgunbabur commented 6 years ago

Yes, I don't see any reason a getter method should modify the organization. Please try removing it.

On Fri, Jan 5, 2018 at 11:00 AM, metincansiper notifications@github.com wrote:

@ozgunbabur https://github.com/ozgunbabur getWith() in https://github.com/PathwayCommons/chibe/blob/master/src/main/java/org/gvt/ layout/BiPaLayout.java#L287 is calling shiftToLastRow(). It seems that this call is the reason of the bug. Shall I remove that call and the method itself as well if it has no other usage?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PathwayCommons/chibe/issues/69#issuecomment-355636943, or mute the thread https://github.com/notifications/unsubscribe-auth/ABUCCx06b_o7-Chqtj4BMrNj-ljKMLqQks5tHnFqgaJpZM4RIsFo .

metincansiper commented 6 years ago

I removed it in PR #71