Skip to content
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.

Sign up for GitHub

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

Jump to bottom

refactor: use contextmanager to handle RC params in controller/plotting.py #310

Merged
merged 1 commit into from
Apr 5, 2023
Merged

refactor: use contextmanager to handle RC params in controller/plotting.py #310

merged 1 commit into from
Apr 5, 2023

Conversation

hollandjg
Copy link
Member

Description

Replace global modification of matplotlib rc_params (which controls plot styling) with a context manager

Type of change:

  • refactor

Features:

  • make it so that only the plots generated from the plotting module are affected by the rc_params updates (rather than the whole python session)

Questions:

  • Any readability updates we need here?
  • Do we rely on these updates anywhere else?

@hollandjg hollandjg requested a review from gtdang March 24, 2023 17:31
@hollandjg hollandjg marked this pull request as ready for review March 24, 2023 17:47
@hollandjg hollandjg requested a review from musslick as a code owner March 24, 2023 17:47
gtdang
gtdang approved these changes Mar 29, 2023
View reviewed changes
Copy link
Collaborator

@gtdang gtdang left a comment

Choose a reason for hiding this comment

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

Nice! This is cool!

@hollandjg hollandjg changed the base branch from feat/controller-refactor-plotting to main April 5, 2023 12:17
@hollandjg hollandjg force-pushed the refactor/plotting-rc_params-contextmanager branch from 8a144ee to f198939 Compare April 5, 2023 12:17
@hollandjg hollandjg added this pull request to the merge queue Apr 5, 2023
Merged via the queue into main with commit e2cd52c Apr 5, 2023
@hollandjg hollandjg deleted the refactor/plotting-rc_params-contextmanager branch April 5, 2023 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtdang gtdang gtdang approved these changes

@musslick musslick Awaiting requested review from musslick

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hollandjg @gtdang