search

USGS-R / gsplot

plotting foundation for timeseries reporting
Other
6 stars 14 forks source link

append without losing timezones in strip_pts #413

Closed lindsayplatt closed 7 years ago

lindsayplatt commented 7 years ago

Working on #285

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 79.631% when pulling de0767d7281415ada9f89ad833c6dd2f7b325042 on lindsaycarr:master into 8a1eb23d1675410186085daaa36ba4495eda0e2b on USGS-R:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.1%) to 79.631% when pulling de0767d7281415ada9f89ad833c6dd2f7b325042 on lindsaycarr:master into 8a1eb23d1675410186085daaa36ba4495eda0e2b on USGS-R:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.4%) to 79.13% when pulling 6ba68c52b6e99fc5956f61f4a65889583096d1ee on lindsaycarr:master into 8a1eb23d1675410186085daaa36ba4495eda0e2b on USGS-R:master.

lindsayplatt commented 7 years ago

@jread-usgs @ldecicco-USGS merge-ready?

ldecicco-USGS commented 7 years ago

So, I merged this, then remembered...it's always good to see that the gsplotIntro.html file has at least a minor update (because we show the date). Then we know that all the vignette and (ideally) readme plots look the same.

lindsayplatt commented 7 years ago

Except that we don't commit the html file, so how could we tell if there was a difference?

ldecicco-USGS commented 7 years ago

Why not? I do...

ldecicco-USGS commented 7 years ago

Oh, I see @jread-usgs added it to the git ignore...I have no idea why. I consider it a part of the tests. I'm taking it off the git ignore

ldecicco-USGS commented 7 years ago

No, actually he's right. It's ignored from the vignette folder, but not the "proper" location in inst/doc. So, before each PR...you should be doing:

devtools::build_vignettes()

and knitting the Readme.Rmd.

lindsayplatt commented 7 years ago

just ran it - it did not actually change anything. I think it's because none of our examples have timezones, so they were showing up just fine.

ldecicco-USGS commented 7 years ago

I didn't expect it to change anything...just mentioning that it's probably a good idea to always include it in your (and all of our) pull requests. At least the date up top changed though, right?

https://rawgit.com/ldecicco-USGS/gsplot/master/inst/doc/gsplotIntro.html

right under the author list, there's the date.

lindsayplatt commented 7 years ago

oh yes - that changed. Worth making a new PR for though?

jordansread commented 7 years ago

Agree @ldecicco-USGS, since we can't do visual tests, we should bug each other in PR reviews to include the knits

ldecicco-USGS commented 7 years ago

Not worth another PR (I've actually got it in mine now....)...just mentioning for the future.