ropensci / tidyqpcr

quantitative PCR analysis in the tidyverse
https://docs.ropensci.org/tidyqpcr/
Other
51 stars 18 forks source link

Maintenance edits and minor upgrades, v1.1 #200

Open ewallace opened 6 months ago

ewallace commented 5 months ago

@ramiromagno is there any chance you would be interested in reviewing this pull request?

ramiromagno commented 5 months ago

I'd be happy to.

ramiromagno commented 5 months ago

Hi @ewallace,

How do you want to go about this?

ewallace commented 5 months ago

Hi @ramiromagno , sorry for my slow reply.

I think comments here in the thread would be easiest, for this version. If you think some bigger changes are needed, then a separate pull request.

ramiromagno commented 5 months ago

Okay! Looking into it.

ramiromagno commented 5 months ago

Hi @ewallace,

NEWS

The current NEWS file is giving a NOTE:

> Problems with news in 'NEWS.md':
> No news entries found.

So I would reformat it like this:

# v1.1 (2024-05)

## Maintenance Upgrades for R 4.4.0
- Base pipe `|>` rather than magrittr pipe `%>%`
- New functions in `display_plate_helpers` to facilitate displaying plate plans with varied information
- Removal of superseded `.data$` syntax
- Verification that all tests pass
- General update to v1.1

# v1.0 (2022-06)

## Changes
- Removed plot helper functions `scale_..._nice` and `scale_loglog` from tidyqpcr, now available in the [scales package](https://scales.r-lib.org) using `label_log` and similar functions. Older code may need to change `scale_y_log10nice` to `scale_y_log10(labels = scales::label_log())`.

# v0.5 (2022-05)

## Improvements
- Enhanced documentation and testing
- Reorganized `display_plate` function for greater flexibility; older code will need to use `display_plate_qpcr` to ensure `sample_id` and `target_id` info displays.

# v0.4 (2022-01)

## Improvements
- Enhanced documentation and argument-checking

# v0.3 (2021-10)

## Testing
- Unit tests now cover over 75% of tidyqpcr code

# v0.2 (2021-06)

## Publications
- [tidyqpcr blogpost in eLife labs](https://elifesciences.org/labs/f23e268f/tidyqpcr-quantitative-pcr-analysis-in-the-tidyverse)

# v0.1 (2020-08)

## Features
- Added relative quantification (delta delta Cq) with function `calculate_deltadeltacq_bytargetid`
- Included a vignette illustrating the function with a small data set from a 96-well plate

# v0.0 (2020-06)

## Upgrades
- Major upgrades that break previous code: all function and variable names changed to snake case (lower case with underscores)
- Specific changes:
  - `sample_id` for nucleic acid sample (replaces Sample or SampleID)
  - `target_id` for primer set/probe (replaces TargetID or Probe)
  - `prep_type` for nucleic acid preparation type (replaces Type)
  - `cq` for quantification cycle (replaces Cq or Ct)
- Users should upgrade old analysis code by case-sensitive search and replace

magrittr's pipe to base R pipe

All good.

Deprecation of the .data pronoun

This deprecation is actually only for the so-called tidyselections (e.g., indicating column names). In other contexts is not deprecated, see this thread (Lionel's comments towards the end): https://github.com/r-lib/tidyselect/issues/169.

Examples

Change from:

deltacq_df |>
        dplyr::group_by(target_id) 

into

deltacq_df |>
        dplyr::group_by("target_id") 

but keep .data here:

        dplyr::mutate(
            deltadelta_cq =.data$ddcq_factor *
                (.data$delta_cq - .data$ref_delta_cq),
            fold_change   = 2 ^ .data$deltadelta_cq
        )
ewallace commented 4 months ago

@ramiromagno thank you for the review.

I can change the NEWS.md formatting.

Regarding the .data pronoun - good catch. Still, if all the tests are passing and the functions and vignettes working, is there any merit in changing it back? Or we just go forward with the edits?

ramiromagno commented 4 months ago

Are you sure devtools::check() runs fine?