-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
BUG: viz plot window's 'title' argument showed no effect. #12828
base: main
Are you sure you want to change the base?
Conversation
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴 |
Looks reasonable @shristibaral ! Can you add a
and then add your name to |
Added: Bugfix-rst
I have added updated docs file |
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.
Pushed a tiny commit to add title
a few more places and document it in the docstrings properly. Will mark for merge-when-green, but might take a couple of hours because we'll want to wait for #12832 to get CIs green first.
if not title: | ||
title = sub_info | ||
else: | ||
title = f"{title}, {sub_info}" |
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.
I don't think this is the right behavior. I don't like that there is no way to prevent subject name from appearing. One concrete case where you don't want to show it: if your subject names aren't de-identified, and you want to show an STC plot in a public presentation/demo.
Most (all?) other places in our codebase, if we have some sort of default/automatic title when title=None
, then passing title="whatever"
would completely replace the default title. I think we should do likewise here.
if not title: | |
title = sub_info | |
else: | |
title = f"{title}, {sub_info}" | |
title = title if title is not None else sub_info |
Title for the figure and the subject name. If None, only the subject name will be | ||
used. |
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.
this controls window title (in the GUI titlebar), not figure title (text on the figure image itself). So this text should be clarified, and also be more clear that the passed title
will be prepended to the subject name. However, see other comment below; I'm not convinced that is the right behavior.
Title for the figure and the subject name. If None, only the subject name will be | |
used. | |
Title for the figure; will be prepended to the subject name (separated by a comma). | |
If ``None``, only the subject name will be used. |
viz plot window's 'title' argument showed no effect. Fix:
if title is given, title along with subject's name will be displayed on plot window title, if title is none then subject name will be displayed as title.
Reference issue
Example: Fixes #1234.
What does this implement/fix?
Explain your changes.
Additional information
Any additional information you think is important.