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

Overwrote methods concerning subdivision of edges in a matching covered graph #39650

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

janmenjayap
Copy link
Contributor

@janmenjayap janmenjayap commented Mar 8, 2025

The objective of this issue is to overwrite the methods pertaining to subdivision of edges in a matching covered graph.

More specifically, this PR aims to overwrite the following methods:

  • subdivide_edge() | Subdivide an edge k times.
  • subdivide_edges() | Subdivide k times edges from an iterable container.

This PR shall address the methods related to subdivision of edge(s) in matching covered graphs.

Fixes #38216.
Note that this issue fixes a small part of the mentioned issue.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

Nothing as of now.

cc: @dcoudert.

Copy link

github-actions bot commented Mar 8, 2025

Documentation preview for this PR (built with commit 823c6a1; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@janmenjayap janmenjayap changed the title Implemented methods concerning subdivision of edges in a matching covered graph Overwrote methods concerning subdivision of edges in a matching covered graph Mar 9, 2025
sage: G.get_matching()
[(0, 3, None), (1, 2, None)]
sage: G.subdivide_edges(G.edges(), 2)
sage: G.edges()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example is failing on some CI.
To avoid that: use a smaller example, don't show the list of edges, but the number of edges, etc.

for edge in edges:
if isinstance(edge, tuple):
if len(edge) <= 1:
raise ValueError('need more than 1 value to unpack')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should also print the edge to clarify the error message.

raise ValueError('expected an iterable of edges, but got a non-iterable object')

for edge in edges:
if isinstance(edge, tuple):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sometimes, we use list or frozenset, etc. and not only tuples.


if len(edge) == 2:
u, v = edge
if not self.has_edge(u, v):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be tested at the same time than the format of edges, no ?

Comment on lines +3892 to +3894
M.add_edges(
[(new_vertices[i * k + j], new_vertices[i * k + j + 1], l) for j in range(0, k - 1, 2)]
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

                M.add_edges((new_vertices[i * k + j], new_vertices[i * k + j + 1], l)
                            for j in range(0, k - 1, 2))

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

Successfully merging this pull request may close these issues.

On Decompositions, Generation Methods and related concepts in the theory of Matching Covered Graphs
2 participants