Skip to content

Fix filter_name type consistency between getter and setter methods#599

Draft
Copilot wants to merge 4 commits intodiagnostic-activate-conveniencefrom
copilot/sub-pr-598
Draft

Fix filter_name type consistency between getter and setter methods#599
Copilot wants to merge 4 commits intodiagnostic-activate-conveniencefrom
copilot/sub-pr-598

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 4, 2025

Description

Addresses feedback from #598 regarding inconsistent type annotations for filter_name parameter.

Changes:

  • Updated get_attr_from_every_element type annotation from str | None to str | tuple[str] | None to match set_attrs_on_every_element
  • Fixed filter logic to properly handle string equality vs tuple membership (prevents substring matching bug when using single string)
  • Updated docstrings to clarify parameter accepts single name or tuple of names

Before (buggy):

# "d" would incorrectly match "d1" due to substring check
filter_name is None or element.name in filter_name

After:

filter_name is None
or (isinstance(filter_name, str) and element.name == filter_name)
or (isinstance(filter_name, tuple) and element.name in filter_name)

Motivation and Context

Original PR: #598
Triggering review: #598 (comment)

  • I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)

Checklist

  • I have updated the changelog accordingly (required).
  • My change requires a change to the documentation.
  • I have updated the tests accordingly (required for a bug fix or a new feature).
  • I have updated the documentation accordingly.
  • I have reformatted the code and checked that formatting passes (required).
  • I have have fixed all issues found by flake8 (required).
  • I have ensured that all pytest tests pass (required).
  • I have run pytest on a machine with a CUDA GPU and made sure all tests pass (required).
  • I have checked that the documentation builds (required).

Note: We are using a maximum length of 88 characters per line.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits December 4, 2025 13:35
Co-authored-by: jank324 <17012833+jank324@users.noreply.github.com>
Co-authored-by: jank324 <17012833+jank324@users.noreply.github.com>
Co-authored-by: jank324 <17012833+jank324@users.noreply.github.com>
Copilot AI changed the title [WIP] WIP address feedback on name filter for Segment attribute Fix filter_name type consistency between getter and setter methods Dec 4, 2025
Copilot AI requested a review from jank324 December 4, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants