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

Ask people to test vignettes #28

Open
5 of 25 tasks
andrew-edwards opened this issue Aug 2, 2023 · 7 comments
Open
5 of 25 tasks

Ask people to test vignettes #28

andrew-edwards opened this issue Aug 2, 2023 · 7 comments
Assignees
Labels
priority Issue to do next

Comments

@andrew-edwards
Copy link
Member

andrew-edwards commented Aug 2, 2023

Big thanks for agreeing to be guinea pigs for our vignettes. You get to tick off a box when you've done one!

If you aren't interested in looking at them all just let me know so we can keep track (edit this issue if you like)- I kind of put everyone to look at all of them, which is probably too much to ask of you. Travis and Andy will go through each other's vignettes first, to give them a first edit.

Steps:

  1. Install pacea - see instructions at https://github.com/pbs-assess/pacea#installation.
  2. Either:
    (a) if you did build_vignettes = TRUE in the installation, you can just do
    vignette(package = "pacea") to list the vignettes, and then
    vignette("buoys") etc. to view the html for each one in turn. This opens the .html, to go through the .Rmd files (which is what I prefer) you can find them in your R library folder, something like R/R-4.3.0/library/pacea/doc/ (where you'll need to know where the library folder resides on your computer).
    (b) Or just download a vignette .Rmd file into a local folder (see below for the link and click on the 'download raw file' icon in the top right.)
    When we release pacea we will provide built vignettes on GitHub, which will simplify all this, but for now we need to do it this way.
  3. Read through the html, maybe trying some commands. Or run the vignette's .Rmd code (click knit in RStudio or see the top of the file for the render command; or you can first ctrl-enter the R code line-by-line, which I find is a more interactive way of understanding a vignette than just running a bunch of code).
  4. Please add any comments below (which means other testers will see them and not have to duplicate any obvious ones) and we will update the vignettes. Suggestions for improvements, clarifications, simplifications etc. welcome. If time then do dig into the help files for various functions.

Travis added some comments below for Andy's vignettes, but they haven't been incorporated yet. The ROMS and satellite data ones are not quite ready yet.

Andrea - chatting to Charles the other day we noticed some of the buoy data are not updated (but they are on your site), it's because I stuck with the DFO source, and just need to switch that for some of them - see #33.

Climatic and oceanographic indices https://github.com/pbs-assess/pacea/blob/main/vignettes/indices.Rmd

  • Travis
  • Kelsey
  • Andrea
  • Brianna
  • Chris

SST buoy data https://github.com/pbs-assess/pacea/blob/main/vignettes/buoys.Rmd

  • Travis
  • Kelsey
  • Andrea
  • Brianna
  • Chris

Fish populations - recruitment/biomass output from stock assessments https://github.com/pbs-assess/pacea/blob/main/vignettes/fish_populations.Rmd

  • Travis
  • Kelsey
  • Andrea
  • Brianna
  • Chris

ROMS output vignettes (not finished yet)

  • Andy
  • Kelsey
  • Andrea
  • Brianna
  • Chris

Satellite SST (not finished yet)

  • Andy
  • Kelsey
  • Andrea
  • Brianna
  • Chris

Add any more here.

@andrew-edwards
Copy link
Member Author

My indices.Rmd one is ready for you to have a look over, @travistai2

@travistai2
Copy link
Contributor

Comments for indices.Rmd vignette
by Travis Tai

  • Provide other date format options from base R as.Date

as.Date("1999-01-01")
as.Date("01/01/99", format = "%d/%m/%y")
as.Date("Mar 31, 2001", format = "%b %d, %Y")
?as.Date # for more date format options

  • Plotting options for values vs anomalies
    Help files for plot.pacea_index, value argument needs what types of values can be plotted ("value", "anomaly"...)

  • Plotting style for indices.
    I was a bit confused here. The default plot(oni) is a line plot with blue and red colours, but does the default change to a bar plot with plotting smooth_over_year, but the style = "red_blue" is the default?
    Playing around with it more, some indices default with a line plot, some with a bar plot.
    The style argument default has oni without a black line, but red_blue has the outline black line. What is the default argument (i.e. red blue without a black line)

@travistai2
Copy link
Contributor

Comments for buoy_sst vignette
-by Travis Tai

Should there be an explanation as to why there are some gaps in the data? Perhaps in the Details of ?buoy_sst and the vignette.

I think the second paragraph of Details for ?buoy_sst is a bit wordy which reads off as a bit confusing. Maybe "If you need data as up-to-date as possible then contact Andrew Edwards and he can update the package, or clone the developer package contents from GitHub." This is already assuming they know how to do so and the operations of using GitHub.

@travistai2
Copy link
Contributor

I don't have any comments for the fish_populations vignette

@andrew-edwards
Copy link
Member Author

Comments for indices.Rmd vignette by Travis Tai

  • Provide other date format options from base R as.Date

as.Date("1999-01-01") as.Date("01/01/99", format = "%d/%m/%y") as.Date("Mar 31, 2001", format = "%b %d, %Y") ?as.Date # for more date format options

  • I have the lubridate-pacea-series() function already (but it's only called from within a plotting function, and isn't fully general). I hadn't put full dates in pdo etc to emphasise they are monthly values (i.e. 2023-08-01 implies 1st August), though you have to pick a day anyway when plotting. So maybe I should add a Date column, for all time series, so that then we can have an calc_anomaly() function to calculate anomalies, that works on many series (e.g. hake recruitment, calculated bccm average temperature over a region, etc.). And have standardised plotting. Sound good? @travistai2

@briannamwright
Copy link

briannamwright commented Sep 9, 2023

  • Comment for "Climatic and Oceanographic Indices: there appears to be no help file for the Southern Oscillation Index - typing in '?soi' elicits the message "No documentation for ‘soi’ in specified packages and libraries: you could try ‘??soi’"
    Thanks - somehow missed that one.

@briannamwright
Copy link

briannamwright commented Sep 9, 2023

Also for the Climatic Indices the default plotting labels for axes and tick marks are super tiny - I know that is all readily changeable in plotting options but maybe the default size could be a bit larger?

  • Andy to look into: they come out fine in the vignettes, I think it's hard to make them always look fine. But as you say, they are tweakable.

@andrew-edwards andrew-edwards added the for Version 1.0.0.0 Needs completing before official release label Oct 23, 2023
@andrew-edwards andrew-edwards added priority Issue to do next and removed for Version 1.0.0.0 Needs completing before official release labels Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority Issue to do next
Projects
None yet
Development

No branches or pull requests

3 participants