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

Support for ets class objects #231

Merged
merged 6 commits into from
Feb 11, 2016
Merged

Support for ets class objects #231

merged 6 commits into from
Feb 11, 2016

Conversation

mitchelloharawild
Copy link
Contributor

Added support for ets class objects

Added support for ets class objects
Added support for ets class objects
@mitchelloharawild mitchelloharawild changed the title Master Support for ets class objects Dec 7, 2015
@@ -41,6 +41,7 @@ New classes supported by the `pander` generic S3 method:
* lrm
* summary.rms
* polr/summary.polr
* ets
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this into separate category, since technically this change is not in pander 0.6

@RomanTsegelskyi
Copy link
Contributor

Thanks for the PR! In general, great job, just few minor style comments (inlined). Also to fix the failing test, you need to abb forecast to recommented packages in DESCRIPTION.

mitchelloharawild and others added 2 commits December 8, 2015 11:47
NEWS and styling fixes, added forecast to DESCRIPTION.
@mitchelloharawild
Copy link
Contributor Author

Thanks for the feedback, I've made some changes.

Checking output of tests, may have miscounted.
Update: found issues and resolved them.

@codecov-io
Copy link

Current coverage is 77.61%

Merging #231 into master will increase coverage by +0.16% as of 54ce51f

Powered by Codecov. Updated on successful CI builds.

@daroczig
Copy link
Member

daroczig commented Dec 9, 2015

@mitchelloharawild, thank you very much for the pull request & fixed, and thanks for @RomanTsegelskyi for the comments.

I generally like the output, but not sure if all parts are important -- can you please share your opinion on that? First, we return the call, which already includes the "type of ets" and "lambda" as well, so the next two lines seem to be redundant to me. Maybe we can have either the rather technical call or the other two lines, no? If we keep the call, was it intentional to have a line break between Call: and the formula? It will be rendered on the same line after calling pandoc -- as double line breaks are required in markdown for a new paragraph.

And maybe we could add a dollar sign around the sigma^2 and use LaTeX markup so that it renders much nicer in PDF/HTML/docx.

What do you think?

@daroczig
Copy link
Member

As I did not hear from you about the above questions, I suppose everything is all right and should remain as is :) Thank you both, again! Merging.

daroczig added a commit that referenced this pull request Feb 11, 2016
@daroczig daroczig merged commit 33be385 into Rapporter:master Feb 11, 2016
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.

4 participants