Skip to content

Conversation

mantepse
Copy link
Contributor

This provides aliases n_vertices, n_edges and n_faces, and uses them throughout the docstring EXAMPLES.

The old aliases num_verts,num_edges and num_faces are kept. Also, they are kept in the code base, to avoid touching too many files.

@mantepse
Copy link
Contributor Author

cf #40887, #40914, #40917

@mantepse
Copy link
Contributor Author

I did not touch the method names in the graph backends. Should I change them and add aliases for consistency?

Copy link

github-actions bot commented Sep 29, 2025

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

@fchapoton fchapoton self-assigned this Sep 30, 2025
@dcoudert
Copy link
Contributor

I did not touch the method names in the graph backends. Should I change them and add aliases for consistency?

Yes, that would be better. Thanks.

@mantepse
Copy link
Contributor Author

There are about 50 files in total that use num_verts() or num_edges(), many of them are already touched by this PR (which touches about 25 files). If you want to, I can of course change also the usage of num_xxx in the codebase, it is not a big deal, I am happy either way.

Copy link
Contributor

@dcoudert dcoudert left a comment

Choose a reason for hiding this comment

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

LGTM.

changes in the codebase can be done in future PR if needed.
Thanks.

@mantepse
Copy link
Contributor Author

changes in the codebase can be done in future PR if needed. Thanks.

Thank you! Small request: could you (and @fchapoton, and whoever else might be interested) please make a decision. It would be easier for me to do it now (happily in a different PR) than in a few weeks.

@dcoudert
Copy link
Contributor

For the graph module (at least), yes you can do changes in the codebase as well. We have the aliases for compatibility. Thanks.

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.

3 participants