Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enforce circular layout to be circular (and not elliptic)? #430

Closed
maximelucas opened this issue Jul 25, 2023 · 6 comments
Closed

enforce circular layout to be circular (and not elliptic)? #430

maximelucas opened this issue Jul 25, 2023 · 6 comments
Labels

Comments

@maximelucas
Copy link
Collaborator

Right now, the circular layout may appear elliptic depending on the aspect ratio of the axis (see below). We enforce the aspect to be "equal". Do we want that? networkx does not apparently.

Screenshot 2023-07-23 alle 18 42 01

Modified from originally posted by @thomasrobiglio in #415 (comment)

@leotrs
Copy link
Collaborator

leotrs commented Jul 25, 2023

I'd suggest we do enforce it.

@maximelucas
Copy link
Collaborator Author

I'd tend to enforce it too. The user can still manually change the aspect ratio later if they want something specific.
A simple ax.set_aspect("equal") should suffice.

@maximelucas
Copy link
Collaborator Author

Actually, I don't think anything can be done inside xgi.circular_layout() because it only returns positions, but no axis.
The position values are correct. They end up being plotted as an ellipsis later when using xgi.draw() is used, an axis is created, and the aspect of the axis is determined. So it can be enforced externally like this:

fig, ax = plt.subplots()
pos = xgi.circular_layout(H)
xgi.draw(H, pos=pos, ax=ax)
ax.set_aspect("equal")

I'm not even sure we can enforce this automaticall inside xgi.draw() because that function is just passed pos but does not know about which layout it corresponds to. Am I missing anything?

@thomasrobiglio
Copy link
Collaborator

@maximelucas all you are saying is correct. What if we just point that out in the docs/tutorials and keep the function as it is?
The ax.set_aspect() could be enforced in the drawing functions in the case ax is None but I don't think that solves the issue, considering also that it would affect all the other layout + clash with the default aspect ratio of axes in matplotlib

@maximelucas
Copy link
Collaborator Author

Agreed, was also thinking we could mention this in the docs of xgi.circular_layout(H) and tutos.

Yea enforcing this in draw for ax is None does not seem optimal. The only way could be to actually check the positions of the nodes inside draw: if they are on a circle (output of circular_layout) then enforce aspect. Let me see how short that could be.

@maximelucas
Copy link
Collaborator Author

We've just discussed with Alice and Nicholas and decided to do the following:
Add an aspect argument to draw(), the value of which will be passed to ax.set_aspect(aspect, "datalim").
By default, we will set it to "equal" so that the the circular layout is a circle. This will also be the default for any other layout/pos.

maximelucas added a commit that referenced this issue Jul 27, 2023
* feat: enforce equal aspect for circular layout #430

* changed implementation of set_aspect
tlarock pushed a commit to tlarock/xgi that referenced this issue Aug 3, 2023
* feat: enforce equal aspect for circular layout xgi-org#430

* changed implementation of set_aspect
maximelucas added a commit that referenced this issue Aug 4, 2023
* Added weights option to to_line_graph function.

* Added tests for weighted line graph implementation.

* Added s=2 test.

* Added encapsulation dag code.

* Added encapsulation dag tests.

* Removed unnecessary combinations import.

* Updated docstring.

* included functions from publication (#400)

* feat: added shuffle_hyperedges + tests

* feat: added shuffle_hyperedges + tests

* fix: filenames

* fix: tests

* style: black and isort

* feat: added function node_swap + test

* style: black and isort

* fix: error from python 3.11

* attempt at listing the available statistics (#405)

* attempt at listing the available statistics

* fixing previous errors

* Attempt at implementing suggestions by nich

* second attempt at displaying the titles

* another trial

* titles in bold

* update of the panel for dihypergraph stats

* minor fixes

* change the hierarchical structure

* minor change

* moving MultiDi-stats in the right place

* adding decorators to the admonition box

* moving up the decorators

* up-version

* feat: enforce equal aspect for circular layout #430 (#432)

* feat: enforce equal aspect for circular layout #430

* changed implementation of set_aspect

* Refactor draw module (#435)

* refact: draw module

* style: black and isort

* fix #331 (#438)

* Added the ability for users to access the optional arguments of NetworkX layout functions. (#439)

* Added another test.

* Refactored relations to subset_types. Exposed and documented empirical relations filtering function. Documentation improvements.

* Update xgi/convert/encapsulation_dag.py documentation

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Fix typo in xgi/convert/encapsulation_dag.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Updates to documentation.

* Minor refactor.

---------

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>
Co-authored-by: Thomas Robiglio <83019028+thomasrobiglio@users.noreply.github.com>
Co-authored-by: Nicholas Landry <nicholas.landry.91@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants