Skip to content

MNT: use more-inclusive language in _trirefine.py#32040

Open
dhananjaya-kularatne wants to merge 1 commit into
matplotlib:mainfrom
dhananjaya-kularatne:fix-trirefine-terminology
Open

MNT: use more-inclusive language in _trirefine.py#32040
dhananjaya-kularatne wants to merge 1 commit into
matplotlib:mainfrom
dhananjaya-kularatne:fix-trirefine-terminology

Conversation

@dhananjaya-kularatne

Copy link
Copy Markdown

PR summary

Closes #32039.

Renames master/slave terminology in _refine_triangulation_once
(both in-line comments and variable names) to major/minor, which
preserves the original meaning (the triangle with the higher vs. lower
index at a shared edge) without using outdated terminology.

This is a pure renaming change — no logic was modified. All existing
tests in test_triangulation.py pass unchanged (50 passed), including
test_trirefine, test_trirefine_masked, and
test_trirefiner_fortran_contiguous_triangles, which exercise this
function directly.

Naming follows the discussion in #32039; happy to adjust if maintainers
land on different terminology.

AI Disclosure

I used Claude (Anthropic) as a guide throughout this contribution — for
identifying the issue, planning the rename scope, drafting the replacement
code, and walking through the Git/build/test workflow. All code was
reviewed, tested locally, and understood by me before submitting.

PR checklist

  • "closes [MNT]: Use more-inclusive language in _trirefine.py #32039" is in the body of the PR description
  • new and changed code is tested (existing tests pass; this is a
    pure rename with no behavior change)
  • Plotting related features are demonstrated in an example (N/A —
    not a plotting feature)
  • New Features and API Changes are noted (N/A — internal rename,
    no public API change)
  • Documentation complies with general and docstring guidelines (N/A
    — no docstrings were touched, only internal comments/variables)

@github-actions

Copy link
Copy Markdown

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks. We also ask that you please finish addressing any review comments on this PR and wait for it to be merged (or closed) before opening a new one, as it can be a valuable learning experience to go through the review process.

You can also join us on discourse chat for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@story645

Copy link
Copy Markdown
Member

Thanks for the pr. Can you please explain why you think major/minor is a good replacement for master/slave in this context?

@dhananjaya-kularatne

Copy link
Copy Markdown
Author

I went with major/minor largely following your comment on #32039, it preserves the original meaning (the triangle with the higher index vs. the one with the lower index at a shared edge) without implying anything about the geometry of the triangles themselves. Terms like "higher"/"lower" or "upper"/"lower" felt like they could be misread as referring to position or size rather than index order, which was the exact ambiguity you flagged. major/minor also reads naturally in phrases like mask_majors, minors_majors_apex, etc., without becoming awkward.
Happy to switch to something else if you'd prefer a different convention, just let me know.

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.

[MNT]: Use more-inclusive language in _trirefine.py

2 participants