Rename have_optional_dependency

[kratman]

Description

Renames have_optional_dependency() to import_optional_dependency()

Fixes #3864

Type of change

• Bug fix (non-breaking change which fixes an issue)

Key checklist:

• No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
• All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[agriyakhetarpal]

Thanks, @kratman! For next time we should look at removing such functions from the public API – I don't think it is being used by anyone currently and does not provide any useful functionality to users either.

[kratman]

Thanks, @kratman! For next time we should look at removing such functions from the public API – I don't think it is being used by anyone currently and does not provide any useful functionality to users either.

Yeah, python's handling of private methods makes me miss C++

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (0767c8d) to head (cb32f63).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3866      +/-   ##
===========================================
- Coverage    99.60%   99.60%   -0.01%     
===========================================
  Files          259      259              
  Lines        21272    21270       -2     
===========================================
- Hits         21188    21186       -2     
  Misses          84       84              

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

[Saransh-cpp]

Thanks! We should also look into adding __all__ to pybamm modules; not in this PR though.

[kratman]

@arjxn-py, @Saransh-cpp I saw this in latexify()

        if sympy:
            from pybamm.expression_tree.operations.latexify import Latexify

I assume this no longer make sense here either. There used to be an "import_optional_dependency" above it

[arjxn-py]

Looks good to me, thanks

[Saransh-cpp]

I assume this no longer make sense here either. There used to be an "import_optional_dependency" above it

Yes! IIRC we did do some hacky stuff to make sympy optional, maybe this was left behind.

[arjxn-py]

I assume this no longer make sense here either. There used to be an "import_optional_dependency" above it

Yes, please also consider removing the import of import_optional_dependency in base_model as it should be unused now plus we can also import latexify on top now.
Edit: Already done in #3848

[agriyakhetarpal]

We should also look into adding all to pybamm modules; not in this PR though.

I agree, xref: #2427 for public and private API discussions