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

Remove/replace nicePlot feature #1

Open
fontikar opened this issue Mar 3, 2021 · 10 comments
Open

Remove/replace nicePlot feature #1

fontikar opened this issue Mar 3, 2021 · 10 comments

Comments

@fontikar
Copy link
Collaborator

fontikar commented Mar 3, 2021

Need to consider whether this is still useful. @dfalster suggests not. But would be good to offer a plotting alternative.

@fontikar
Copy link
Collaborator Author

@dwarton agrees with @dfalster on this one. He adds that users can plot the sma.obj however they like with whatever package they want. Shall we also close this one?

@dfalster
Copy link
Member

Hi @fontikar, If we're removing the feature, we can't just close, first need to remove the code. Then revaluate the plot.sma function. I think we should provide some guidance on how to easily plot, using both baseR and ggplot. Can you look into it see more and then discuss?

@fontikar
Copy link
Collaborator Author

fontikar commented Aug 25, 2022

Ah yes apologies @dfalster I wasn't clear in my previous comment. I didn't mean close the issue as away to remove the feature. From what I gathered @dwarton suggested to just leave the feature alone as it works as a quick visualisation and matches figures presented in the original MEE paper. I agree perhaps a tutorial on plotting with baseR or ggplot2 would be valuable! I will investigate these and will come back with some options!

@dfalster
Copy link
Member

dfalster commented Aug 25, 2022

Got it. In that case

  • review other all functions should be exported. Perhaps fewer than current
  • develop ggplot equivalent, as existing code is already base R

@fontikar
Copy link
Collaborator Author

Sounds great! I think we can definitely reduce number of exported functions for sure!

@dwarton
Copy link
Collaborator

dwarton commented Aug 25, 2022

did I suggest leaving it? I can't remember... I think plot of an sma object should do something, maybe that is what I meant.

@fontikar
Copy link
Collaborator Author

Have a very simple version of the function going, can produce the smatr plots for single S(MA), next is to expand to multiple groups 317d0fc My plan is to use group_by compute the line and the plot but grabbing group info from obj$coef in a function is proven tricky, maybe a for loop but I think map is possible!

@dfalster
Copy link
Member

Good start Fonti.

For multiple groups, The output of Sma already includes a table of pars, I think it's called groupsummary (?)?

Happy to review together.

fontikar added a commit that referenced this issue Oct 19, 2022
fontikar added a commit that referenced this issue Oct 19, 2022
fontikar added a commit that referenced this issue Oct 19, 2022
@fontikar
Copy link
Collaborator Author

fontikar commented Oct 19, 2022

Managing well to expand to multiple groups with common slope, next steps to get add the ability to varying elevation and slope sma(y~x*group)

image

@fontikar
Copy link
Collaborator Author

fontikar commented Oct 21, 2022

Great, current functions work well for:

  • sma(y~x*group) use cases
  • sma(y ~ x+group, type = "elevation")
  • sma(y ~ x+group, type = "shift")

image

Next steps:

  • Setting a default for sma(longev ~ lma, slope.test = 0.8, data=leaflife), obj$group == "all"
  • Use cases when (S)MAs are forced through the origin
  • Residual plots
  • qq plots

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

No branches or pull requests

3 participants