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.

Sign up for GitHub

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

Jump to bottom

Documentation: Add a small clarification in the vignette derive_basetype_records #2545

Open
cpiraux opened this issue Oct 31, 2024 · 1 comment
Open

Documentation: Add a small clarification in the vignette derive_basetype_records #2545

cpiraux opened this issue Oct 31, 2024 · 1 comment
Assignees
@jimrothstein
Labels
documentation Improvements or additions to documentation

Comments

@cpiraux
Copy link

cpiraux commented Oct 31, 2024

Please select a category the issue is focused on?

Function Documentation

Let us know where something needs a refresh or put your idea here!

I propose adding "where either BASE or BASEC are also non-null" (from ADaM IG 1.3) to the sentence in the vignette: "BASETYPE must be non-null on all records of any type for that parameter".

When BASETYPE is defined by period, it does not make sense to populate BASETYPE for screening records where BASE is missing. Adding this clarification can help users to derive BASETYPE correctly.

Text in the vignette:
image

BASETYPE definition in ADaM IG 1.3:
image
@cpiraux cpiraux added the documentation Improvements or additions to documentation label Oct 31, 2024
@sadchla-codes sadchla-codes self-assigned this Nov 11, 2024
@jimrothstein jimrothstein self-assigned this Dec 12, 2024
jimrothstein added a commit that referenced this issue Dec 14, 2024
@jimrothstein
#2545, #2604 fix typo, document()
2e1f425
bms63 added a commit that referenced this issue Dec 22, 2024
@bms63
chore: #2545 docs
982d7f6
bms63 added a commit that referenced this issue Dec 22, 2024
@jimrothstein @bundfussr @bms63
Closes #2604 documentation clarification vignette derive basetype rec…
2f70ceb 
…ords (#2610)

* # 2545 documentation - derive_basetype_records

don't think I follow the logic;
Thought:  lawyers do it like this.
i) criteria, AND exactly ONE of

iia) criteria2, OR
iib) criteria 2b

blah blabh...

* # 2545, 2604  fix typo

line 233:

~~PARAMCAT1~~
PARCAT1

* #2545, #2604  fix typo,  document()

* Update R/derive_basetype_records.R

Co-authored-by: Stefan Bundfuss <[email protected]>

* chore: #2545 docs

---------

Co-authored-by: Stefan Bundfuss <[email protected]>
Co-authored-by: Ben Straub <[email protected]>
@bms63
Copy link
Collaborator

bms63 commented Jan 6, 2025

@jimrothstein I think this got updated. Maybe the issue number wasn't correct in the Pull Request Title and did not auto-close??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jimrothstein jimrothstein

Labels
documentation Improvements or additions to documentation
Projects
admiral (sdtm/adam, dev, ci, template, core)
Status: In Progress
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jimrothstein @bms63 @cpiraux @sadchla-codes