Make get_spatial_var private

[Akhil-Sharma30]

Description

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

1. pybamm.QuickPlot.create_gif removed the round brackets around the optional keyword.
2. pybamm.QuickPlot.dynamic_plot made the Args as optional.
3. pybamm.QuickPlot.get_spatial_var made private.
4. pybamm.QuickPlot.plot Documented dynamic arg.

Fixes #3754

Type of change

Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

• New feature (non-breaking change which adds functionality)
• Optimization (back-end change that speeds up the code)
• 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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.59%. Comparing base (33371b0) to head (ac4ec8c).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3755   +/-   ##
========================================
  Coverage    99.59%   99.59%           
========================================
  Files          259      259           
  Lines        21353    21353           
========================================
  Hits         21266    21266           
  Misses          87       87           

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

[agriyakhetarpal]

3. pybamm.QuickPlot.get_spatial_var made private.

This will require a note in the CHANGELOG too as a breaking change (even if this function was not being used by a lot of users)

[Akhil-Sharma30]

3. pybamm.QuickPlot.get_spatial_var made private.

This will require a note in the CHANGELOG too as a breaking change (even if this function was not being used by a lot of users)

Added in the changelog.md

[kratman]

@Akhil-Sharma30 Just a note, I would suggest that you don't force push if you can avoid it. It overwrites history on git and makes it harder to see what was reviewed.

[Akhil-Sharma30]

@Akhil-Sharma30 Just a note, I would suggest that you don't force push if you can avoid it. It overwrites history on git and makes it harder to see what was reviewed.

Okay sure I will take care of that in future.

[Saransh-cpp]

See https://github.com/pybamm-team/PyBaMM/pull/3752/files#r1463064059

[Saransh-cpp]

See #3752 (review)

Could you please also change the PR title to reflect that this is deprecating something and not fixing a bug? Thanks!

[kratman]

@Akhil-Sharma30 Are you still working on this and #3752

[Akhil-Sharma30]

@Akhil-Sharma30 Are you still working on this and #3752

Yes, I was busy with some other work.

[kratman]

@Saransh-cpp What is left on this one? Just deprecation warnings and changing the PR title?

[Saransh-cpp]

And changelog, that should be it.

[arjxn-py]

@Akhil-Sharma30 Are you still working on this?

[kratman]

The changes seem minor and since the function only appears in this file, I don't think the deprecation warning is needed

As far as I can tell the changes were made. We can open a new ticket for the deprecation warning if it is a major concern before the release. It seems like the function is not widely used, so it is probably not needed.