Skip to content

Conversation

szhorvat
Copy link
Member

No description provided.

@szhorvat
Copy link
Member Author

@ntamas, I need help here. How do I pass a graph object to a free function? I'm getting this error from PyIGraph_ToCGraph:

In [7]: ig.align_layout(g, lay.coords) --------------------------------------------------------------------------- TypeError Traceback (most recent call last) Cell In[7], line 1 ----> 1 ig.align_layout(g, lay.coords) TypeError: expected graph object, got <class 'list'> In [8]: g Out[8]: <igraph.Graph at 0x1039b7050> 
@szhorvat
Copy link
Member Author

Never mind, I figured it out.

@szhorvat szhorvat force-pushed the feat/align-layout branch 3 times, most recently from f25ee95 to 1236f42 Compare July 26, 2025 14:59
@szhorvat
Copy link
Member Author

@ntamas It seems that the default layout on my system is not auto but fruchterman_reingold. I see this in ig.config["plotting.layout"]. I can't figure out for my life where this was saved, why, and how I can change it. I have no .igraphrc.

I suspect that @BeaMarton13's difficulties with Cairo being the default plot backend have to do with a saved configuration as well.

@szhorvat
Copy link
Member Author

One of the gallery examples changes and saves the layout:

https://python.igraph.org/en/stable/tutorials/configuration.html#sphx-glr-tutorials-configuration-py

Does building the docs on our system, and this building gallery examples, permanently change the config? That would be a problem.

@ntamas
Copy link
Member

ntamas commented Jul 26, 2025

Does building the docs on our system, and this building gallery examples, permanently change the config?

Yes, it probably does, and indeed it's a problem, but to be honest the whole idea of having a configuration file for a library is the root cause of the problem here. This was a mistake from the early days and I deeply regret it, but it's probably here to stay with this version of the Python interface. It won't happen with the new Python interface.

Are you sure you don't have an .igraphrc file in your home? On my machine it's there:

>>> from igraph.configuration import get_user_config_file >>> get_user_config_file() '/Users/tamas/.igraphrc'
@szhorvat
Copy link
Member Author

szhorvat commented Jul 27, 2025

@ntamas, some questions regarding this PR:

  • What is the proper way to take a Graph as the input, in a function implemented in C? I see at least 3 ways: (1) use O! and igraphmodule_GraphType, then pick the igraph_t pointer out. (2) Use PyIGraph_ToCGraph() — seems the simplest (3) Use igraphmodule_PyObject_to_igraph_t() as I do now — seems the most consistent with how things are already done elsewhere
  • Where do we make use of alignment automatically? Options:
    1. Only in layout_auto()
    2. In all "organic" layout functions, such as fr, kr, etc. (but of course not in layouts such as tree, bipartite, circle, random, etc.)
    3. In all "organic" layout functions, but also add a parameter align=True to them to make it possible to disable alignment. If we do this, should an align parameter be added to all functions, including layout that shouldn't be aligned (where it's either ignored to set to False by default)? This would provide a consistent interface, ensuring that the same parameters can be passed along to any layout.
  • Do we align only smallish graphs, e.g. up to a vertex count of 1000? Alignment should be relatively cheap (it's linear time), but it's mostly pointless for messy large graphs, except for making sure that the plot is more wide than tall, which does have some value.

My preference is to:

  • Align in all organic layouts, but do not add an align parameter for the simple reason of time saving.
@szhorvat
Copy link
Member Author

Regarding configuration,

  • It seems that it affects tests as well (they passed on my machine).
  • What is then the true default plotting backend? The logic is too complex, and I'm somewhat confused. It should be matplotlib, but it seems to me that the default is still Cairo. Should we not switch this to matplotlib on develop?
  • Perhaps that gallery example should be updated not to save the configuration to a file. We can remove ig.config.save() and simple mention that it can be used instead of running it by default. What do you think?
@szhorvat szhorvat force-pushed the feat/align-layout branch from 1236f42 to cb7dc69 Compare July 27, 2025 03:04
@ntamas
Copy link
Member

ntamas commented Jul 28, 2025

What is the proper way to take a Graph as the input, in a function implemented in C?

Oh so we do have an igraphmodule_PyObject_to_igraph_t? Use that. I was looking for this when I was fixing the umap_compute_weights() function but did not find it - maybe it's on develop only.

Bottom like: use igraphmodule_PyObject_to_igraph_t. Fishing out the pointer by hand is error-prone, and PyIGraph_ToCGraph() is a function that is meant to resemble Python's own C API conventions (CamelCase function names, Py prefix etc) so and it is really meant for other Python extensions.

Align in all organic layouts, but do not add an align parameter for the simple reason of time saving.

Agreed.

It seems that it affects tests as well (they passed on my machine).

Yes. That's another reason why it's a terrible idea in the first place.

What is then the true default plotting backend?

AFAIK it's still cairo, according to igraph/configuration.py, which has this line in the schema definition:

"plotting.backend": {"default": "cairo"}

Again, if I were to do this today (assuming that I still wouldn't be convinced that a configuration system for a library is a bad idea), I would do it with a dedicated library like pydantic, which automatically takes care of validation and whatnot. But the configuration system was created in the early days, way before pydantic and similar libraries.

Should we not switch this to matplotlib on develop?

The default should be "whatever is available on the user's machine and whatever fits the current environment (notebooks or not), preferring matplotlib over cairo", but I don't have the resources to implement it. Switching to matplotlib unconditionally is a breaking change, and I have mostly avoided breaking changes so far.

To be honest, with the plans for the new Python interface I don't really feel like changing the defaults right now. There are two options: either there will be funding for the project in the future (in which case I'd rather focus my efforts on the new Python interface instead of changing things in the old Python interface and potentially breaking things for the end users), or there won't be any funding, in which case I will not have the resources to deal with people coming to complain about a unilateral breaking change (especially because the two backends do not have feature parity). There has always been an option to change the plotting backend and people who needed matplotlib so far did make the change anyway.

Perhaps that gallery example should be updated not to save the configuration to a file.

Sounds good - please send a PR and I'll merge it.

@szhorvat
Copy link
Member Author

To be honest, with the plans for the new Python interface I don't really feel like changing the defaults right now.

Alright, then let's use the config system to change the default to matplotlib for the community detection tutorial, but not save the config.

@BeaMarton13, can you please do this? See https://python.igraph.org/en/stable/tutorials/configuration.html#sphx-glr-tutorials-configuration-py for an example.

@szhorvat szhorvat force-pushed the feat/align-layout branch from cb7dc69 to 1f48b47 Compare July 28, 2025 13:24
@szhorvat szhorvat marked this pull request as ready for review July 28, 2025 13:25
@szhorvat
Copy link
Member Author

@ntamas, once again, I have to ask if you can help update the few failing tests ... I expect these must somehow hard-code properties of specific layouts. Unfortunately I wasn't able to run these specific tests locally due to missing dependencies. Running Python tests is extremely painful as it keeps accessing online resourcs, which keep timing out and barely give me 10 kB / s ...

@szhorvat
Copy link
Member Author

image
@szhorvat szhorvat force-pushed the feat/align-layout branch from e4c18e6 to 183e6a4 Compare July 28, 2025 13:51
@ntamas ntamas merged commit 9a3e610 into develop Aug 2, 2025
49 of 55 checks passed
@ntamas ntamas deleted the feat/align-layout branch August 2, 2025 10:12
@ntamas
Copy link
Member

ntamas commented Aug 2, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants