Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Agriya Khetarpal <[email protected]>
  • Loading branch information
arjxn-py and agriyakhetarpal authored Nov 9, 2023
1 parent ec963d1 commit dd8a6f2
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 10 deletions.
8 changes: 4 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,23 +106,23 @@ PyBaMM utilizes optional dependencies to allow users to choose which additional

PyBaMM provides a utility function `have_optional_dependency`, to check for the availability of optional dependencies within methods. This function can be used to conditionally import optional dependencies only if they are available. Here's how to use it:

Optional Dependencies should never be imported at the module level, but always inside methods. For example:
Optional dependencies should never be imported at the module level, but always inside methods. For example:

```
def use_pybtex(x,y,z):
pybtex = have_optional_dependency("pybtex")
...
```

While importing a specific attribute instead of whole module:
While importing a specific module instead of an entire package/library:

```
```python
def use_parse_file(x,y,z):
parse_file = have_optional_dependency("pybtex.database","parse_file")
...
```

This allows people to (1) use PyBaMM without importing Optional dependency by default and (2) configure module dependent functionality in their scripts, which _must_ be done before e.g. `print_citations` method is first imported.
This allows people to (1) use PyBaMM without importing optional dependencies by default and (2) configure module-dependent functionalities in their scripts, which _must_ be done before e.g. `print_citations` method is first imported.

## Testing

Expand Down
4 changes: 2 additions & 2 deletions pybamm/expression_tree/unary_operators.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ def _unary_new_copy(self, child):

def _sympy_operator(self, child):
"""Override :meth:`pybamm.UnaryOperator._sympy_operator`"""
sympy_Gradient = have_optional_dependency("sympy.vector.operators","Gradient")
sympy_Gradient = have_optional_dependency("sympy.vector.operators", "Gradient")
return sympy_Gradient(child)


Expand Down Expand Up @@ -403,7 +403,7 @@ def _unary_new_copy(self, child):

def _sympy_operator(self, child):
"""Override :meth:`pybamm.UnaryOperator._sympy_operator`"""
sympy_Divergence = have_optional_dependency("sympy.vector.operators","Divergence")
sympy_Divergence = have_optional_dependency("sympy.vector.operators", "Divergence")
return sympy_Divergence(child)


Expand Down
4 changes: 2 additions & 2 deletions pybamm/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,12 +359,12 @@ def have_optional_dependency(module_name, attribute=None):
return imported_attribute # Return the imported attribute
else:
# Raise an ImportError if the attribute is not available
raise ImportError(f"Optional dependency {module_name} is not available. See https://docs.pybamm.org/en/latest/source/user_guide/installation/index.html#optional-dependencies for more details.")
raise ModuleNotFoundError(f"Optional dependency {module_name} is not available. See https://docs.pybamm.org/en/latest/source/user_guide/installation/index.html#optional-dependencies for more details.")
else:
# Return the entire module if no attribute is specified
return module

except ImportError:
except ModuleNotFoundError:
# Raise an ImportError if the module or attribute is not available
if attribute:
raise ImportError(f"Optional dependency {module_name} is not available. See https://docs.pybamm.org/en/latest/source/user_guide/installation/index.html#optional-dependencies for more details.")
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_expression_tree/test_unary_operators.py
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,8 @@ def test_not_constant(self):
def test_to_equation(self):

sympy = have_optional_dependency("sympy")
sympy_Divergence = have_optional_dependency("sympy.vector.operators","Divergence")
sympy_Gradient = have_optional_dependency("sympy.vector.operators","Gradient")
sympy_Divergence = have_optional_dependency("sympy.vector.operators", "Divergence")
sympy_Gradient = have_optional_dependency("sympy.vector.operators", "Gradient")

a = pybamm.Symbol("a", domain="negative particle")
b = pybamm.Symbol("b", domain="current collector")
Expand Down

0 comments on commit dd8a6f2

Please sign in to comment.