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

Review: Global/big picture formatting revisions #96

Closed
ttimbers opened this issue Mar 22, 2021 · 13 comments
Closed

Review: Global/big picture formatting revisions #96

ttimbers opened this issue Mar 22, 2021 · 13 comments

Comments

@ttimbers
Copy link
Contributor

No description provided.

@ttimbers ttimbers changed the title Global/big picture formatting revisions asked for in review Review: Global/big picture formatting revisions Mar 22, 2021
@leem44
Copy link
Contributor

leem44 commented Mar 28, 2021

Reviewer E

  • placement of figures is confusing throughout this text (e.g. Sections 1.62, 4.5.2). figures are often far from what they described; sometimes even directly under the wrong code chunk.
    • recommend forcing appropriate figure placement or, at minimum, using more specific language of “In Figure X, we see…” instead of “in the plot below”.
  • code frequently spills out over the textboxes. may need to split some commands across more lines or change the font.
  • In 3.4:
    • More weird spacing of figures and text

@trevorcampbell
Copy link
Contributor

trevorcampbell commented Mar 28, 2021

Reviewer D

  • background colour used for the figures is not always consistent. Some figures use the default grey theme of ggplot, while others don’t. The grey theme is not the best default so perhaps the authors could consider using a more visually appealing theme.
  • the book is a bit long for an introduction -- suggest removing the statistical inference chapter
    • I disagree. We teach a 1-semester course on this. Each chapter corresponds to a week.
  • Also, all code or output included in this section (or in the book) should be formatted to fit the page width

@trevorcampbell
Copy link
Contributor

trevorcampbell commented Mar 28, 2021

Reviewer A

  • the color schemes should be carefully chosen so that colorblind readers have no problem reading them. Double encoding could also be used to help with this.
  • From a typesetting perspective, there are some problems with the figures in the PDF (but not the HTML version). In the PDF, the figures often fall too far from the discussion or code snippet. There is certainly some tweaking to be done in typesetting, but I also think that it is important to explicitly point to the figure being discussed for the print edition.
  • the book is of the right length for an introduction
  • be sure no code gets cut off and that figures are placed as close to the text as possible.
  • use bold for package names and end function names with () as suggested in the preface
    • I think we should do the former, not the latter
  • I'd like clearer typography for notes. Does CRC have a standard format? A title in each "note box" would help
  • Both "dataset" and "data set" are used in the book. One should be chosen for consistency
  • make sure URLs and code don't spill off the page
  • Canadian vs american spellings (e.g. consistent with spelling of colour (or color))
  • it would be great if all figures had captions to help reader make reference to them
  • similarly, throughout the text, make explicit reference to figure caption labels. see e.g. p171 "training set:" where there's a colon, an image should be after, but the PDF renderer moves everything around so it doesn't line up properly any more.
  • Be consistent between scatterplot and scatter plot, dataset data set
  • make sure spacing on math typesetting is right (e.g. see top of p 209)
  • p145: (re: rotating the 3d image) This caption would apply to the online version, but not the print version.
  • p163: the figures are hard to follow (the flow of them) because they get jumbled in the PDf version. in the HTML version it's fine. actual comment: I expected to see the scatterplot as the first graphic. If you want the reader to see these in print, be sure to point to them specifically in the text.

@trevorcampbell
Copy link
Contributor

trevorcampbell commented Mar 28, 2021

Trevor

  • we say "Professor Claus Wilke" and "Grolemund & Wickham" when referring to textbooks. We should go through and make sure we reference other resources in a consistent way
  • Some of the section numbering is weird. E.g. in section 4.4 "Refining the viz", the next subsection is "4.4.0.1" . Should be "4.4.0". I think this is because in the HTML version, these numbers aren't printed, and we chose the (sub)^n-section style for the nicest printout. In a PDF the numbers are printed, so we should just stick with the right nesting level.
  • generally speaking, figures should appear at the tops of pages. some places that doesn't make sense, but in general that should be the style
  • make sure the script for the videos doesn't make course-specific references (e.g. the very first video in Ch1 starts by making a vague reference to DSCI100)

@trevorcampbell
Copy link
Contributor

trevorcampbell commented Mar 28, 2021

Edit this comment directly to summarize / synthesize the major (important) comments across reviewers. If there's already the same comment below, add the reviewer to the parenthetical list at the beginning.

Synthesis:

  • Figure placement overall is confusing and broken. We need to add reference anchors to all figures, and then make sure to reference those anchors in the text.
  • plot background colours should be consistent (some are ggplot grey, some are not)
  • Code blocks often spill over the edge, math is formatted poorly.
  • some reviewers thought it was too long (D), some thought just right (A). I think it's fine.
  • consistency of spelling (color vs colour, data set vs dataset, scatter plot vs scatterplot)
  • clearer / nicer typography of "Note:" blocks including a block title of some sort
  • choose colourblind friendly colourschemes for all chapters (e.g. Ch10 is particularly bad)
  • make consistent the way we reference textbooks and other sources (see Trevor comment above)
  • make the section numberings more sensible (e.g. 4.4 should be followed by 4.4.0, not 4.4.0.1)
  • move all figures to the top of the page where possible (some that are "in the flow" can be left as part of the text)
  • make sure the monologue scripts for the videos doesn't make course-specific references (e.g. the very first video in Ch1 starts by making a vague reference to DSCI100)
  • ensure all figures are in colour
  • improve code formatting so code arguments and strings are not greyed out.
  • be consistent whether (or not) we put parentheses after code-typeset function names. Reviewer C (and Tiffany) suggest to not use parens.
  • don’t use ‘/‘ in code
  • Change all video links to be stable links, that we can redirect behind the scenese

@ttimbers
Copy link
Contributor Author

Reviewer F

  • The book is black/white but refers to colour highlighting in JP

@ttimbers
Copy link
Contributor Author

ttimbers commented Mar 29, 2021

Reviewer C

  • This is a minor point, but there are places where parts of the code are grayed out – I presume this is a result
    of converting colorized code to black and white. Given that some of the plots are in color, I presume the
    book will be printed full-color, in which case the color could be retained for the code. If that isn’t to be
    done, please convert all code to simple black and white.
  • Are you putting parens after function names? I recommend not doing it, even though it’s common practice, but consistency is what matters most.
  • Please don’t use ‘/‘ in code like this - people will think it’s a division sign. (Yes, I’ve done this, and yes, this happened…) noted in section 1.6.3
  • greying out of argument names and strings in code chunks is confusing: I presume the graying out is inherited from code colorization, but it’s confusing

This was referenced Jul 27, 2021
@leem44
Copy link
Contributor

leem44 commented Aug 7, 2021

Adding from #92
Classification1 chapter:

  • we should also show 2-3 views of the 3D plot given that it will need to be static.

@leem44
Copy link
Contributor

leem44 commented Aug 19, 2021

How should we display links to video content, such as this line from Ch 13: "Thus, we recommend you change the default shell to Bash by opening the terminal (how-to video) and typing:"

@trevorcampbell
Copy link
Contributor

trevorcampbell commented Aug 20, 2021

In the Jupyter chapter (where there were other videos) we just replaced them with static screenshot images. I would be fine with that strategy throughout. But in that chapter we made that choice because there was a lot of video demo stuff, and if we didn't replace it with something the chapter would essentially become "go watch these youtube videos...".

So in this case, since it's just a little "hey if you don't already know how to do this thing, here it is, now come back and finish the chapter", I would also be fine with linking to some other static online resource (so you could e.g. just put a link to Tiffany's youtube video).

@ttimbers what do you think?

@ttimbers
Copy link
Contributor Author

ttimbers commented Sep 30, 2021

From @trevorcampbell - "Figure 4.16 on page 102 does not highlight the artifacts the same way the web version does... did we accidentally load the same image twice?"

No, it does not in the PDF - this was something we need to fix. It is not simple nor straightforward due the the svg. Will move this to the formatting pass.

@ttimbers
Copy link
Contributor Author

ttimbers commented Oct 2, 2021

A couple of things that I didn't comment on but we should add to the formatting pass:

sometimes the caption has periods at the end, sometimes it doesn't -- we should add this to the style guide which one we want
the plots and code coming off the page + the placement of figures -- add to formatting pass

@trevorcampbell
Copy link
Contributor

All of these things are either handled, already in the style guide, or in open issues (which may add them to style guide).

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