Plot update

[mbsabath]

Added labels to the base Zelig graph axes

[christophergandrud]

Thanks @mbsabath

My main concern for this is ensuring that we've tested all cases and that they make sense. Maybe in a fork, could you compile all of the vignettes and that should implement the new labels for most plots. We will then need to manually check these.

[christophergandrud]

Also, from the CI check, it looks like the pull request doesn't pass CRAN check due to undefined global variables n.y1. You just need to start the offending function with n.y1 <- NULL

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@b3cefb3). Click here to learn what that means.
The diff coverage is 66%.

@@            Coverage Diff            @@
##             master     #297   +/-   ##
=========================================
  Coverage          ?   88.24%           
=========================================
  Files             ?       50           
  Lines             ?     4159           
  Branches          ?        0           
=========================================
  Hits              ?     3670           
  Misses            ?      489           
  Partials          ?        0

Impacted Files	Coverage Δ
R/model-timeseries.R	87.65% <100%> (ø)
R/plots.R	77.96% <63.82%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b3cefb3...2619baf. Read the comment docs.

[mbsabath]

The most recent commit took care of all the check notes on my machine

[christophergandrud]

Just looking over the plots created in the README for this pull request and thinking about ways forward.

In the following plot I like the "Density" y-axis label. That seems like a clear win. But in this plot the Predicted Values are labeled "Expected Values" on the x-axis. That should be changed.

In the next plot the bottom two figures have the x-axis label "Value", which isn't really informative.

Overall

The new y-axis labels seem good. But maybe the new x-axis labels are a bit inconsistent and potentially not informative (thus worsening the data-ink ratio). I think the issue is that in many cases the old plot titles (e.g. "First Differences: E(Y|X1) - E(Y|X)") describe the x-axis. So adding new x-axis labels is in most cases redundant.

I guess the question then is whether we should just move the current plot titles to be the x-axis labels?

[mbsabath]

I think that's a valid question. In my mind, there are two options for this. One of which is to change the X Label to be something basic like "Value" and leaving the title as it is, or in the event no xlabel is supplied, moving the default to be the label for the graph. Both of these solutions would be pretty easy to implement, and mostly come down to preference. I personally think that using the title as the default x label makes the most sense, but am not set on it. Do you have a preference?

[christophergandrud]

I'm adverse to the the value solution as this is non-informative. Perhaps moving the title to the x-axis sense. Lets ask James later today.

[christophergandrud]

Great. I'll take more of a look this weekend.

[christophergandrud]

Overall pretty nice. I noticed one oddity for Ordered Logistic Regression (and likely similar):

"Y Value" could really be "Outcome" as well.