search

igraph / rigraph

igraph R package
https://r.igraph.org
544 stars 200 forks source link

feat!: Stop accepting setting `layout` as a graph attribute if it is a matrix #1303

Open krlmlr opened 6 months ago

krlmlr commented 6 months ago

Closes #775.

@maelle: Can you please pick up from here?

aviator-app[bot] commented 6 months ago

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes. Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This pull request is currently open (not queued).

How to merge

To merge this PR, comment /aviator merge or add the mergequeue label.

See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.
aviator-app[bot] commented 6 months ago

This pull request failed to merge: this PR is in draft state. Remove the blocked label to re-queue.

Additional debug info: PR was marked as draft after queueing

aviator-app[bot] commented 6 months ago

This pull request can't be queued because it's currently a draft.

szhorvat commented 6 months ago

Then there will need to be a convenient way to enter the layout information as vertex attributes (x and y, or x, y and z).

I'm not exactly sure what features exist for this.

There's e.g. add_layout_() which actually adds a graph attribute ...

krlmlr commented 6 months ago

Accept a vertex attribute layout that is decomposed into x, y and z, and return a layout attribute when asked and if x and y exist?

szhorvat commented 6 months ago

Something of that sort, yes.

The problem is that the function that compute layouts return matrices.

But in order to store layouts in graphs properly, we need to split these matrices into x, y and maybe z components.

For convenience, there should be some easy way to convert between the two representations. In particular, there should be a very easy way to compute a layout (produced as a matrix) and store it in the graph as x, y vertex attributes. Perhaps the add_layout_() function should be updated to do precisely this. This is a "mildly breaking" change.

(If you ask me, the best solution would be if coordinate tuples could be stored in a single "position" vertex attribute, but unfortunately the C core inly supports scalars at the moment, not vectors/tuples, as attribute values associated with individual vertices. I hope we get the new grant from the CZI and can work on improving this.)

krlmlr commented 6 months ago

@szhorvat: Something like 20dc1e4?