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

[Persistence_matrix] Type renaming to be more coherent with Gudhi conventions #1110

Merged
merged 2 commits into from
Jul 29, 2024

Conversation

hschreiber
Copy link
Collaborator

All types are now starting with a capital letter, with no "_type" or other at the end, as discussed in #917:

          So, if I understood well, your preference would be: all types start with a capitalized letter and only the name of object without any "_type" or "_t" at the end? Eg. `dimension_type` ---> `Dimension`.

Originally posted by @hschreiber in #917 (comment)

          Yes on the description of my preference, I think that's mostly what we've been doing so far in Gudhi (with obvious exceptions like value_type for compatibility with the standard library, or Thing_type where "type" is not in the C++ sense but means "kind", "category").

Originally posted by @mglisse in #917 (comment)

Copy link
Member

@mglisse mglisse left a comment

Choose a reason for hiding this comment

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

I didn't check all of them in details, but it looks good to me.
A couple oddities:

  • Boundary_container is a container to describe the content of 1 boundary, while Column_container is a container that contains many columns?
  • Column::Column::iterator (we already had that before this PR)

@hschreiber
Copy link
Collaborator Author

I didn't check all of them in details, but it looks good to me. A couple oddities:

* Boundary_container is a container to describe the content of 1 boundary, while Column_container is a container that contains many columns?

* Column::Column::iterator (we already had that before this PR)

Now that you are saying it, it looks indeed odd. I changed Boundary_container to Boundary_range and the inner Column to Column_support. Is it better?

Copy link
Contributor

@VincentRouvreau VincentRouvreau left a comment

Choose a reason for hiding this comment

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

I did not review them all, but this is the exact role of these code conventions (characteristic_type becomes Characteristic, the syntax says it is a type, and it is clearer)

@VincentRouvreau VincentRouvreau merged commit aa0c4bf into GUDHI:master Jul 29, 2024
7 checks passed
@VincentRouvreau VincentRouvreau added the 3.11.0 GUDHI version 3.11.0 label Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11.0 GUDHI version 3.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants