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

Define __iter__ and/or __contains__ for pybamm.ParameterValues #4438

Open
ejfdickinson opened this issue Sep 13, 2024 · 3 comments
Open

Define __iter__ and/or __contains__ for pybamm.ParameterValues #4438

ejfdickinson opened this issue Sep 13, 2024 · 3 comments
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours feature

Comments

@ejfdickinson
Copy link

Description

The pybamm.ParameterValues is dict-like, and through its __getitem__ and __setitem__ methods allows dict-like key access. However, it does not support the in keyword, which for a dict will treat the keys as an iterable. This means that natural code like:

if "Negative particle radius [m]" in parameter_values:
    # do something

will not work (generally raises a KeyError). It is currently necessary to instead write explicitly:

if "Negative particle radius [m]" in parameter_values.keys():
    # do something

Defining the missing method __iter__ (and __contains__ if preferred) would allow in to work in logical test and for-loop contexts.

Motivation

Code is cleaner if pybamm.ParameterValues looks like a souped-up dict always, not just sometimes. Key-value accessors are expected to iterate over their keys and to allow logical test on the presence/absence of keys. PyBaMM-containing code looks 'asymmetric' compared to code working with actual dict objects due to the requirement to call into pybamm.ParameterValues.keys() specifically.

Was there any historic reason for the decision to exclude these methods?

Possible Implementation

No response

Additional context

No response

@rtimms
Copy link
Contributor

rtimms commented Sep 13, 2024

AFAIK these weren't intentionally excluded.

@martinjrobins
Copy link
Contributor

I just ran into this problem today, please do implement these, it would be very helpful! :)

@agriyakhetarpal agriyakhetarpal added the difficulty: easy A good issue for someone new. Can be done in a few hours label Sep 14, 2024
@kratman
Copy link
Contributor

kratman commented Sep 16, 2024

Yeah I have wanted this as well, but never go around to adding anything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours feature
Projects
None yet
Development

No branches or pull requests

5 participants