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

[DOC] Added example to plot_series & fixed example for plot_lags #3400

Merged
merged 13 commits into from Sep 18, 2022
Merged

[DOC] Added example to plot_series & fixed example for plot_lags #3400

merged 13 commits into from Sep 18, 2022

Conversation

shagn
Copy link
Contributor

@shagn shagn commented Sep 9, 2022

This PR adds an example for the plot_series function and adds a missing import statement for the example of plot_lags.

For all contributions
  • I've added myself to the list of contributors.
  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG] indicating whether the PR topic is related to enhancement, maintenance, documentation, or bug.

@fkiraly
Copy link
Collaborator

fkiraly commented Sep 10, 2022

thanks!

fkiraly
fkiraly previously approved these changes Sep 11, 2022
@fkiraly
Copy link
Collaborator

fkiraly commented Sep 11, 2022

looks ok, but still a draft.

We could merge it like that. Or, suggestions:

  • add yourself to the contributors file, all-contsributorsrc
  • add a line of explanation in the all_estimators example, what does it do?

@shagn
Copy link
Contributor Author

shagn commented Sep 13, 2022

Sure, I will do that

@shagn shagn marked this pull request as ready for review September 13, 2022 20:03
@shagn
Copy link
Contributor Author

shagn commented Sep 13, 2022

I just realised I made a mistake. I did merge the main branch of sktime directly and now the meanwhile introduced changes are also shown. I am not sure how to untangle that the best way... I will have a look tomorrow.

@fkiraly
Copy link
Collaborator

fkiraly commented Sep 14, 2022

??

merging regularly from main to keep your PR branch updated is what you are expected to do in best practice GitHub flow, so all is good!

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Looks good, thanks a lot!

@shagn
Copy link
Contributor Author

shagn commented Sep 17, 2022

Yes, I know. But I thought I should have done it differently, but it is fine. Sorry for the confusion @fkiraly.

@fkiraly fkiraly merged commit e6cefd0 into sktime:main Sep 18, 2022
@fkiraly
Copy link
Collaborator

fkiraly commented Sep 18, 2022

no probs, all great, and thanks for your contribution!

@shagn shagn deleted the doc-plot-functions branch September 18, 2022 21:11
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.

None yet

2 participants