GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Update shared axes processing in plot_time_response by murrayrm · Pull Request #1040 · python-control/python-control

Conversation

@murrayrm

@murrayrm murrayrm commented Aug 13, 2024

Copy link
Copy Markdown
Member

This PR updates the way that tickmarks are labeled on shared axes in time response plots, in response to a comment from @slivingston in #1034. Tickmark labels are now omitted on shared axes, consistent with frequency response plots.

To allow tickmark labels to be added, two new keyword arguments are included in time_response_plot: sharex and sharey. These are passthrough arguments to matplotlib.pyplot.subplots and allow axes sharing to be turned on and off in the same way as bode_plot => frequency and time response plots are now consistent.

Also added unit tests checking for consistency and updated the user documentation and relevant docstrings.

Without tickmark labels With tickmark labels
Version 0.10.0:
plot_gallery-0 10 0-2024-06-20		 Previous to this PR (from #1034):
plot_gallery-0 10 0-191-g373ff11e-2024-08-08
After this PR (using default settings):
plot_gallery-0 10 0-201-gc9a07c13-2024-08-09		 After this PR (using sharex=False):
step_response-sharex_false

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@murrayrm
update shared axes processing in plot_time_response
71f4bc1
@coveralls

coveralls commented Aug 13, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Coverage Status

coverage: 94.694% (+0.001%) from 94.693%
when pulling 9c8ffec on murrayrm:tickmark_labels-08Aug2024
into ecf6a38 on python-control:main.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@slivingston slivingston self-assigned this Aug 14, 2024
slivingston
slivingston reviewed Aug 14, 2024
View reviewed changes
Comment thread control/freqplot.py Outdated
will force the legacy values (mag, phase, omega) to be returned
(with a warning). To obtain just the frequency response of a system
(or list of systems) without plotting, use the
1. Starting with python-control version 0.10, `bode_plot`returns a

@slivingston slivingston Aug 14, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
1. Starting with python-control version 0.10, `bode_plot`returns a
1. Starting with python-control version 0.10, `bode_plot` returns a

This space is important because without it, Sphinx fails to parse the :class: expression.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@slivingston slivingston Aug 14, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

After fixing this, the PR is ready to merge.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@slivingston slivingston Aug 14, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

You may want to commit the figures in the documentation that are affected by this PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@murrayrm
address @slivingston review comments
9c8ffec
@murrayrm murrayrm mentioned this pull request Aug 15, 2024
Version 0.10.1 release plans #1020
Closed
11 tasks done
@murrayrm
murrayrm merged commit 3e9a39e into python-control:main Aug 15, 2024
@murrayrm
murrayrm deleted the tickmark_labels-08Aug2024 branch August 15, 2024 06:31
@murrayrm murrayrm added this to the 0.10.1 milestone Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slivingston slivingston slivingston left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@slivingston slivingston

Labels

None yet

Projects

None yet

Milestone

0.10.1

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@murrayrm @coveralls @slivingston
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.