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

In hyperSpec.Rmd faux_cell is repeatedly overwritten #224

Open
bryanhanson opened this issue Jul 21, 2020 · 5 comments
Open

In hyperSpec.Rmd faux_cell is repeatedly overwritten #224

bryanhanson opened this issue Jul 21, 2020 · 5 comments
Labels
Priority: 3-low Topic: vignette 📗 Related to vignettes of hyperSpec

Comments

@bryanhanson
Copy link
Collaborator

In the course of working on #223 I noticed that chondro is repeatedly overwritten as the vignette progresses. Wouldn't it be better to assign the result to something like chondro.tmp <- to avoid modifying the modifications repeatedly? I'm a little surprised it behaves well.

@GegznaV
Copy link
Collaborator

GegznaV commented Jul 21, 2020

I saw similar issues in several vignettes. It seemed a bit vulnerable, as some modifications in code may create mismatches between the results of code and narration. In some places mismatches were already present.

@cbeleites
Copy link
Owner

cbeleites commented Jul 22, 2020

I think we may distinguish the flow of the data analysis ("narration") from "side notes" about solving thematically related issues that are however, not following the central line of the narration.

  • Along the flow of the data analysis, IMHO the object should be overwritten. After all, that's usual and it is what variables are for: containing values that change over the course of a program.
    This would refer to the vignettes that give data analysis examples, i.e. chondro, laser, flu.

For vignette hyperSpec part of the secret for behaving is that

  • at least at some points the modified chondro object is deleted, so we move back to the version provided by the package.
    Here, replacing by a temporary copy would be good.
  • some changes redo the preprocessing in vignette chondro and are needed for later parts.

But you are right: this is likely confusing for readers.

I'm thinking that we may want to further break the vignette into parts: keep vignette hyperSpec focused on a description of how hyperSpec objects behave and have separate vignettes for "spectroscopic tasks", e.g. correcting the wavelength axis (and we anyways already have the topic "baseline correction")

@bryanhanson
Copy link
Collaborator Author

bryanhanson commented Jul 22, 2020

I may not have been completely clear. We have code that looks like this in the vignette:

chondro <- modify(chondro) #  1
...
chondro <- modify(chondro) # 2
...
chondro <- modify(chondro) # 3

My point is that only # 1 is using chondro, the others use an increasingly modifed chondro. If a user is interested in the line marked # 2 and executes it (and just it, nothing that comes before it) on their own local machine, they will not get the same result as seen in the vignette. This could be alleviated by assigning the result to chondro2 or chondro.tmp or anything. Just have to be careful in a particular section if chondro.tmp is then plotted or further processed we need to plot(chondro.tmp) etc.

@GegznaV
Copy link
Collaborator

GegznaV commented Jul 22, 2020

@cbeleites By "narration" I meant "text that describes plots and other results produced by R code".

For workflow demonstration vignettes, I agree, that this is acceptable. But for vignettes, that demonstrate separate pieces of functionality, this does not seem convenient. Because if code in one section changes, it affects everything bellow.

@bryanhanson Does it seem that this behavior could result in some issues in hyperSpec.Rmd when functions are removed from hyperSpec to other packages?

or chondro.tmp

On code style: I'd prefer using dots only in the function names of S3 methods and using underscores instead of dots in all other object names in the examples/vignettes. (E.g., → chondro_tmp).

@GegznaV GegznaV added the Topic: vignette 📗 Related to vignettes of hyperSpec label Jul 24, 2020
@GegznaV GegznaV changed the title In hyperSpec.Rmd chondro is repeatedly overwritten In hyperSpec.Rmd faux_cell is repeatedly overwritten Aug 4, 2020
@GegznaV
Copy link
Collaborator

GegznaV commented Aug 4, 2020

I updated the title of this issue as chondro is replaced by faux_cell in the vignette.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 3-low Topic: vignette 📗 Related to vignettes of hyperSpec
Projects
None yet
Development

No branches or pull requests

3 participants