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.

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

ENH: Plot Config #27

Merged
merged 11 commits into from
Aug 29, 2024
Merged

ENH: Plot Config #27

merged 11 commits into from
Aug 29, 2024

Conversation

aksharsarvesh
Copy link
Collaborator

Added functionality to the Plot tab such that a user can set up the viewer however they like. The option to add more functions in the future remains, and it would not be that difficult to do so.

archive_viewer/mixins/__init__.py Outdated Show resolved Hide resolved
archive_viewer/mixins/plot_config.py Outdated Show resolved Hide resolved
archive_viewer/mixins/plot_config.py Outdated Show resolved Hide resolved
archive_viewer/mixins/plot_config.py Outdated Show resolved Hide resolved
archive_viewer/mixins/plot_config.py Outdated Show resolved Hide resolved
archive_viewer/mixins/plot_config.py Outdated Show resolved Hide resolved
archive_viewer/mixins/plot_config.py Outdated Show resolved Hide resolved
archive_viewer/mixins/plot_config.py Outdated Show resolved Hide resolved
archive_viewer/mixins/plot_config.py Outdated Show resolved Hide resolved
archive_viewer/mixins/plot_config.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@YektaY YektaY left a comment

Choose a reason for hiding this comment

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

This looks good to me. I have one style note regarding adding docstrings to the PlotConfigMixin class. I can merge in once you have added the documentation and fixed the merge conflict.

from pyqtgraph import ViewBox


class PlotConfigMixin:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Style: Can you add docstrings to all the methods in this class?

for reference on the docstring format we are using:
https://numpydoc.readthedocs.io/en/latest/format.html

@aksharsarvesh
Copy link
Collaborator Author

Added docstrings and fixed merge conflicts. This PlotConfig was self contained to this repo so it should be safe to merge whenever you want it

@YektaY
Copy link
Collaborator

YektaY commented Aug 28, 2024

apologies, looks like a new merge conflict appeared after I merged your other PR

@YektaY YektaY merged commit c4f192b into slaclab:main Aug 29, 2024
0 of 7 checks passed
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