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

Maintenance edits and minor upgrades, v1.1 #200

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

ewallace
Copy link
Collaborator

No description provided.

@ewallace ewallace changed the title Maintenance edits and minor upgrades Maintenance edits and minor upgrades, v1.1 Apr 29, 2024
@ewallace
Copy link
Collaborator Author

ewallace commented May 8, 2024

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

@ramiromagno
Copy link

I'd be happy to.

@ramiromagno
Copy link

Hi @ewallace,

How do you want to go about this?

  • Comments in situ across the code (I think I have to be granted permission)
  • Comments here in the thread
  • A separate PR with suggestions for code changes (requires a fork to my own account)

@ewallace
Copy link
Collaborator Author

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
Copy link

Okay! Looking into it.

@ramiromagno
Copy link

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): r-lib/tidyselect#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
Copy link
Collaborator Author

ewallace commented Jun 4, 2024

@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
Copy link

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

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

Successfully merging this pull request may close these issues.

2 participants