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

Feature Request: Allow more than one unit for required_unit in assert_unit() #468

Closed
bundfussr opened this issue Oct 9, 2024 · 4 comments · Fixed by #471
Closed

Feature Request: Allow more than one unit for required_unit in assert_unit() #468

bundfussr opened this issue Oct 9, 2024 · 4 comments · Fixed by #471
Assignees
Labels
enhancement New feature or request programming

Comments

@bundfussr
Copy link
Collaborator

bundfussr commented Oct 9, 2024

Feature Idea

Allow more than one unit for required_unit in assert_unit() (see pharmaverse/admiral#2513).

Additionally:

  • Make required_unit optional. If not specified, only the uniqueness of the unit in the input dataset is checked.
  • Document the uniqueness check.
  • Document the required variables (PARAMCD) in the description of the dataset argument.

Relevant Input

No response

Relevant Output

No response

Reproducible Example/Pseudo Code

No response

@yurovska
Copy link

@bundfussr In addition to allowing more than one unit for required_unit, it would also be great if we could use assert_unit() with required_unit being optional, so that it only performs the check for multiple units within a parameter, i.e. without a restriction of what that unit is.

@bundfussr
Copy link
Collaborator Author

@bundfussr In addition to allowing more than one unit for required_unit, it would also be great if we could use assert_unit() with required_unit being optional, so that it only performs the check for multiple units within a parameter, i.e. without a restriction of what that unit is.

@yurovska , good idea. I've updated the description.

@manciniedoardo
Copy link
Collaborator

@bundfussr do you have capacity to knock this one out in the next week or two pls? then we can integrate the update within admiralmetabolic ahead of its first release.

@bundfussr bundfussr self-assigned this Oct 30, 2024
@bundfussr
Copy link
Collaborator Author

@bundfussr do you have capacity to knock this one out in the next week or two pls? then we can integrate the update within admiralmetabolic ahead of its first release.

Yes, no problem.

@bundfussr bundfussr moved this from Backlog to In Progress in admiral (sdtm/adam, dev, ci, template, core) Oct 30, 2024
bundfussr added a commit that referenced this issue Oct 31, 2024
bundfussr added a commit that referenced this issue Oct 31, 2024
bundfussr added a commit that referenced this issue Oct 31, 2024
bundfussr added a commit that referenced this issue Oct 31, 2024
@bundfussr bundfussr moved this from In Progress to In Review in admiral (sdtm/adam, dev, ci, template, core) Oct 31, 2024
bundfussr added a commit that referenced this issue Nov 5, 2024
bundfussr added a commit that referenced this issue Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request programming
Development

Successfully merging a pull request may close this issue.

3 participants