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] Implementing plot title for plot_series #4038

Merged
merged 3 commits into from Jan 5, 2023

Conversation

arnavrneo
Copy link
Contributor

Reference Issues/PRs

Fixes #3944

What does this implement/fix? Explain your changes.

This implementation updates plotting.py under utils directory. The function plot_series is updated and now the user can enter the plot title for the figure.

Changes:

  • The function plot_series now has one more parameter: suptitle (default value is None) to which user can give the title of the plot or figure.

Does your contribution introduce a new dependency? If yes, which one?

No.

PR checklist

For all contributions

⬜ I've added myself to the list of contributors.
✅ I've added unit tests and made sure they pass locally.
✅ The PR title starts with either [ENH], [MNT], [DOC], or [BUG] indicating whether the PR topic is related to enhancement, maintenance, documentation, or bug.

@chillerobscuro
Copy link
Contributor

Hey @arnavrneo, this looks like a nice new feature!

Is it necessary to use suptitle instead of title when this function only makes plots with one ax?

Either way I'd suggest calling the argument title just to keep the interface as straight forward as possible, even if we do use suptitle under the hood.

@arnavrneo
Copy link
Contributor Author

@chillerobscuro Thanks!

About suptitle or title, I read in the matplotlib documentation; about those two it stated the following:

suptitle

Add a centered suptitle to the figure.

This adds a self-centered title to the whole figure and not the axes.

title

Set a title for the Axes.
Set one of the three available Axes titles. The available titles are positioned above the Axes in the center, flush with the left edge, and flush with the right edge.

This can be used for different axes.

So, as there was just one ax in the function, I went with suptitle.

And for the arguments, you're right. It should be straight forward as possible. I'll update the code. So, is it good to go with suptitle or title?

@chillerobscuro
Copy link
Contributor

Only difference I can see is suptitle is placed with a bit more spacing above the figure, which I think looks nice. Perhaps a core contributor will give a more weighty decision but IMO this looks good to go now.

Copy link
Collaborator

@miraep8 miraep8 left a comment

Choose a reason for hiding this comment

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

Seems like a pretty straightforward change and it works for me. Congrats on making your first contribution @arnavrneo Will merge tomorrow as long as there are no other change requests in the mean time! You are also welcome/encouraged to add yourself to the list of contributors! :)

@arnavrneo
Copy link
Contributor Author

arnavrneo commented Jan 3, 2023

Thanks!
I've one doubt. For adding myself to the contributors list, do I need to directly edit .all-contributorsrc?

@miraep8 miraep8 merged commit 286e1af into sktime:main Jan 5, 2023
@miraep8
Copy link
Collaborator

miraep8 commented Jan 5, 2023

Hey @arnavrneo - congrats on making your first contribution to sktime!

Apologies I merged this before you were properly added as a contributor, and I am sorry for not answering your questions before! In short - yes - you can add yourself to that document! :) You can either: a) do it yourself in a new PR, b) tack it on to your next PR to sktime or c) I can add you for you! :)

Welcome to sktime again! :) 🎉 🎉 🎉

@arnavrneo arnavrneo deleted the plot-series-title branch January 6, 2023 04:25
@arnavrneo
Copy link
Contributor Author

No worries @miraep8!

Okay, I got it now. I've created #4074. Can you please review it?

Thank you!

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.

[ENH]Plot Title
3 participants