Add Fuzzydict docstring by Akhil-Sharma30 · Pull Request #4846 · pybamm-team/PyBaMM

Akhil-Sharma30 · 2025-02-13T19:07:14Z

Description

In this PR fixed the following bugs in pybamm.util.py:

pybamm.FuzzyDict.copy. needs docstring
pybamm.FuzzyDict. get_best_matches. document key arg
pybamm.FuzzyDict. search. document args. format code properly

Fixes #4845

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #)

Important checks:

Please confirm the following before marking the PR as ready for review:

No style issues: nox -s pre-commit
All tests pass: nox -s tests
The documentation builds: nox -s doctests
Code is commented for hard-to-understand areas
Tests added that prove fix is effective or that feature works

kratman · 2025-02-13T19:20:53Z

+        Retrieves the value associated with a key, handling renamed terms and
+        suggesting closest matches if the key is not found.
+
+    _find_matches(search_key, known_keys)


Private methods should not be documented

Okay I have removed the private methods.

kratman · 2025-02-13T19:22:45Z

+    __getitem__(key)
+        Retrieves the value associated with a key, handling renamed terms and
+        suggesting closest matches if the key is not found.


This is probably better documented as an example rather then the method itself

codecov · 2025-02-13T19:33:01Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.70%. Comparing base (9ec472d) to head (f262534).
Report is 7 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4846   +/-   ##
========================================
  Coverage    98.70%   98.70%           
========================================
  Files          303      303           
  Lines        23335    23335           
========================================
  Hits         23032    23032           
  Misses         303      303

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

Saransh-cpp · 2025-02-13T21:14:12Z

+        This method returns a new FuzzyDict object containing the same key-value pairs
+        as the original dictionary. It ensures that the copied dictionary retains
+        the fuzzy matching behavior.


This looks like an AI generated text 🙁

Anyways, it is redundant. The first sentence makes everything clear.

Suggested change

This method returns a new FuzzyDict object containing the same key-value pairs

as the original dictionary. It ensures that the copied dictionary retains

the fuzzy matching behavior.

Extremely sorry, I have re-written the doc-string and added an example case.

Saransh-cpp · 2025-02-13T21:16:02Z

+    Features:
+    - Fuzzy matching using `difflib.get_close_matches` to suggest the closest keys.
+    - Custom error handling for specific key renamings with informative messages.
+    - Search functionality to find keys containing specific terms.
+    - Custom warnings for deprecated key names.


Does not add anything. I am not against AI, but it is always important to check the results before adding them in.

Suggested change

Features:

- Fuzzy matching using `difflib.get_close_matches` to suggest the closest keys.

- Custom error handling for specific key renamings with informative messages.

- Search functionality to find keys containing specific terms.

- Custom warnings for deprecated key names.

Saransh-cpp · 2025-02-13T21:16:45Z

+    Methods:
+    get_best_matches(key)
+        Returns a list of the best-matching keys for a given input key.
+
+    search(keys, print_values=False)
+        Searches the dictionary for keys containing all specified terms. Prints
+        the results and, optionally, the corresponding values.
+
+    copy()
+        Returns a copy of the FuzzyDict instance.


Methods and their respective docstrings are automatically rendered in the docs

Suggested change

Methods:

get_best_matches(key)

Returns a list of the best-matching keys for a given input key.

search(keys, print_values=False)

Searches the dictionary for keys containing all specified terms. Prints

the results and, optionally, the corresponding values.

copy()

Returns a copy of the FuzzyDict instance.

Saransh-cpp · 2025-02-13T21:19:47Z

+    This class extends the built-in `dict` to provide intelligent key lookup,
+    including approximate string matching and handling of renamed terms. It is
+    useful when working with parameter dictionaries where keys might have slight
+    variations, renamings, or formatting differences.


The first sentence captures the gist here too, so these lines look redundant to me.

Saransh-cpp · 2025-02-13T21:22:00Z

+        as the original dictionary. It ensures that the copied dictionary retains
+        the fuzzy matching behavior.
+
+        Returns:


Suggested change

Returns:

Returns

-------

Saransh-cpp · 2025-02-14T10:31:06Z


        Parameters
-        ----------
+        -------


Suggested change

-------

----------

Saransh-cpp · 2025-02-14T10:31:17Z


        Parameters
-        ----------
+        -------


Suggested change

-------

----------

Saransh-cpp · 2025-02-14T10:31:38Z

+    Methods
+    -------
+    get_best_matches(key)
+        Returns a list of the best-matching keys for a given input key.
+
+    search(keys, print_values=False)
+        Searches the dictionary for keys containing all specified terms. Prints
+        the results and, optionally, the corresponding values.
+
+    copy()
+        Returns a copy of the FuzzyDict instance.
+


Suggested change

Methods

-------

get_best_matches(key)

Returns a list of the best-matching keys for a given input key.

search(keys, print_values=False)

Searches the dictionary for keys containing all specified terms. Prints

the results and, optionally, the corresponding values.

copy()

Returns a copy of the FuzzyDict instance.

Should I remove this section?

Saransh-cpp · 2025-02-14T10:33:46Z

+    Example usage
+    -------
+    ```python
+        >>> params = FuzzyDict({
+        >>> "electrode diffusivity": 1.2,
+        >>> "particle diffusivity": 0.8,
+        >>> "Negative electrode SOC": 0.5,
+        >>> "Open-circuit voltage at 100% SOC [V]": 4.2,
+        >>> })
+
+        >>> print(params["electrode diffusivity"])  # 1.2 (direct access)
+        >>> print(params["particle diffusivity"])   # 0.8 (handles renaming)
+
+        >>> params.search("open circuit voltage")
+
+        >>> params_copy = params.copy()  # Creates a new FuzzyDict instance
+    ```
+    """


You will need to fix this examples as the doctests will fail

Suggested change

Example usage

-------

```python

>>> params = FuzzyDict({

>>> "electrode diffusivity": 1.2,

>>> "particle diffusivity": 0.8,

>>> "Negative electrode SOC": 0.5,

>>> "Open-circuit voltage at 100% SOC [V]": 4.2,

>>> })

>>> print(params["electrode diffusivity"]) # 1.2 (direct access)

>>> print(params["particle diffusivity"]) # 0.8 (handles renaming)

>>> params.search("open circuit voltage")

>>> params_copy = params.copy() # Creates a new FuzzyDict instance

```

"""

Examples

--------

>>> params = FuzzyDict({

>>> "electrode diffusivity": 1.2,

>>> "particle diffusivity": 0.8,

>>> "Negative electrode SOC": 0.5,

>>> "Open-circuit voltage at 100% SOC [V]": 4.2,

>>> })

>>> print(params["electrode diffusivity"])

1.2

>>> print(params["particle diffusivity"])

0.8

>>> params.search("open circuit voltage")

>>> params_copy = params.copy() # Creates a new FuzzyDict instance

"""

Modified the example.

kratman · 2025-03-18T23:23:38Z

@Akhil-Sharma30 Are you still working on this?

Akhil-Sharma30 · 2025-03-20T04:44:45Z

@Akhil-Sharma30 Are you still working on this?

But I have already made the changes asked for, waiting for the review from your end.

Saransh-cpp · 2025-03-20T09:32:00Z

@Akhil-Sharma30 did you forget to push your changes? The comments above are still unresolved.

Akhil-Sharma30 · 2025-03-20T11:32:18Z

@Akhil-Sharma30 did you forget to push your changes? The comments above are still unresolved.

Could you please verify reflected as per #4846.

Saransh-cpp · 2025-03-20T11:34:42Z

I don't see the new commits :(

Please make sure your local branch is synced with the remote branch.

kratman · 2025-05-12T21:32:08Z

@Akhil-Sharma30 Are you still working on this?

Akhil-Sharma30 added 3 commits February 13, 2025 23:12

Add Doc-String for FuzzyDict class

bfa847c

Add doc-string for fuzzyDict.copy func

5c896d6

Fix the indentation

58e937e

Akhil-Sharma30 requested a review from a team as a code owner February 13, 2025 19:07

style: pre-commit fixes

2194231

kratman requested changes Feb 13, 2025

View reviewed changes

Remove the private func

577260a

Saransh-cpp requested changes Feb 13, 2025

View reviewed changes

Saransh-cpp reviewed Feb 13, 2025

View reviewed changes

Akhil-Sharma30 added 2 commits February 14, 2025 10:09

Docs: Re-worked on the FuzzyDict class doc-string

b8abf29

Docs: Improve formatting

f262534

Saransh-cpp requested changes Feb 14, 2025

View reviewed changes

agriyakhetarpal marked this pull request as draft February 17, 2025 02:32

Akhil-Sharma30 and others added 2 commits February 17, 2025 14:35

Add example in the doc-string

b3d8262

style: pre-commit fixes

a352e8f

kratman added the needs-reply Needs further information from the author and may be closed if no response is received label Mar 18, 2025

github-actions Bot removed the needs-reply Needs further information from the author and may be closed if no response is received label Mar 20, 2025

kratman added the needs-reply Needs further information from the author and may be closed if no response is received label May 12, 2025

kratman closed this Jun 4, 2025

	This method returns a new FuzzyDict object containing the same key-value pairs
	as the original dictionary. It ensures that the copied dictionary retains
	the fuzzy matching behavior.

Uh oh!

Conversation

Akhil-Sharma30 commented Feb 13, 2025

Description

Type of change

Important checks:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kratman commented Mar 18, 2025

Uh oh!

Akhil-Sharma30 commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Saransh-cpp commented Mar 20, 2025

Uh oh!

Akhil-Sharma30 commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Saransh-cpp commented Mar 20, 2025

Uh oh!

kratman commented May 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Feb 13, 2025 •

edited

Loading

Akhil-Sharma30 commented Mar 20, 2025 •

edited

Loading

Akhil-Sharma30 commented Mar 20, 2025 •

edited

Loading