Skip to content

Keep FuzzyDict.search labels aligned when the term list has blank entries - #6019

Open
syahra712 wants to merge 3 commits into
pybamm-team:mainfrom
syahra712:fix-fuzzydict-search-blank-terms
Open

syahra712 wants to merge 3 commits into
pybamm-team:mainfrom
syahra712:fix-fuzzydict-search-blank-terms

Conversation

@syahra712

Copy link
Copy Markdown
Contributor

Description

FuzzyDict.search labels its results with the wrong search term when the list of terms contains a blank entry, and the last term is never searched:

>>> model = pybamm.BaseModel()
>>> model.variables = {"Electrolyte concentration": 1, "Electrode potential": 0}
>>> model.variables.search(["", "Electrolyte", "Potenteel"])
Exact matches for '': ['Electrolyte concentration']
No exact matches found for 'Electrolyte'. Best matches are: ['Electrode potential']

Expected:

Exact matches for 'Electrolyte': ['Electrolyte concentration']
No exact matches found for 'Potenteel'. Best matches are: ['Electrode potential']

Cause: blank terms are filtered out of search_keys but kept in original_keys. The per-term loop zips the two lists, so after a blank entry every result is printed against the previous term, and the last term is dropped. The existing mixed-list test passes only because all of its terms match one key together, which takes the earlier "Results for" path and never reaches the zip.

Fix: filter original_keys first and build search_keys from it, so the two lists always line up. The "Results for" message no longer needs its own blank filter.

No linked issue; I found this by reading the code.

Type of change

Bug fix, with a CHANGELOG entry under # [Unreleased] → ## Bug fixes.

Important checks:

  • No style issues: ruff check / ruff format --check on the changed files
  • Tests pass: tests/unit/test_util.py (8 passed). The new case in TestSearch.test_url_gets_to_stdout fails on main and passes with the fix.
  • The documentation builds: not run locally (no docs changes)
  • Tests added that prove fix is effective

AI-assisted (Claude). I reviewed the code and ran the tests.

Blank terms were dropped from search_keys but kept in original_keys, so
zip() paired each result with the wrong term and the last term was never
searched.
@syahra712
syahra712 requested a review from a team as a code owner November 1, 2026 17:22
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

ProjectPyBaMM
Branchfix-fuzzydict-search-blank-terms
Testbedbare-metal

⚠️ WARNING: Truncated view!

The full continuous benchmarking report exceeds the maximum length allowed on this platform.

🐰 View full continuous benchmarking report in Bencher

@codecov

codecov Bot commented Nov 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.22%. Comparing base (8e54b57) to head (18f8c95).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6019      +/-   ##
==========================================
- Coverage   98.23%   98.22%   -0.01%     
==========================================
  Files         350      350              
  Lines       36173    36173              
==========================================
- Hits        35533    35532       -1     
- Misses        640      641       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

This branch has not been deployed

No deployments
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.

1 participant