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

Better option checking #3303

Open
rtimms opened this issue Aug 31, 2023 · 2 comments · May be fixed by #4707
Open

Better option checking #3303

rtimms opened this issue Aug 31, 2023 · 2 comments · May be fixed by #4707
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve

Comments

@rtimms
Copy link
Contributor

rtimms commented Aug 31, 2023

The options checking doesn't always work as expected when options are tuples. For example, the following code performs checks if the option "particle size" is "distribution", but "particle size" can be a tuple, in which case the test doesn't run

        if options["particle size"] == "distribution":
            if options["lithium plating"] != "none":
                raise NotImplementedError(
                    "Lithium plating submodels do not yet support particle-size "
                    "distributions."
                )

The options should be converted to tuples first (where appropriate) and then checks performed on each index.

@rtimms rtimms added difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve labels Aug 31, 2023
@Saswatsusmoy
Copy link

Can i get the path for the same code?

@Saswatsusmoy
Copy link

@rtimms Can you please review this and let me know if any more changes are required in the code.

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 priority: low No existing plans to resolve
Projects
None yet
4 participants