Skip to content

Small simplifications to streamplot. #20739

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

Merged
merged 1 commit into from
Jul 28, 2021
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jul 25, 2021

cm.get_cmap already handles None input, no need to do it again here.

In get_integrator and _integrate_rk12, track x and y together rather
than separately, and the xyt[1:] slicing works even if xyt is empty.

PR Summary

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

Comment on lines 481 to 484
if stotal > minlength > 0:
return [*zip(*xy_traj)]
elif minlength == 0:
return [], []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I'm +/-0 on packing x, y together. It is shorter in some places, but for example this construct is much harder to understand.

Copy link
Contributor Author

@anntzer anntzer Jul 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way to make this actually simpler is to make get_integrator return an (N, 2) array rather than a (2, N) array which is also consistent with what we do everywhere else, but basically means switching to a private _get_integrator with the new convention, so let's also deprecate the old one. (So that's no longer "just" maintenance, as there's a deprecation now.)

cm.get_cmap already handles None input, no need to do it again here.

In get_integrator and _integrate_rk12, track x and y together rather
than separately, and the `xyt[1:]` slicing works even if `xyt` is empty.
@timhoffm timhoffm added this to the v3.5.0 milestone Jul 28, 2021
@timhoffm timhoffm merged commit 2ad9ec6 into matplotlib:master Jul 28, 2021
@anntzer anntzer deleted the streamclean branch July 28, 2021 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants