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

Code for formats where there is an x.y is currently not creating all combinations within xpt_validate() function. #234

Closed
sophie-gem opened this issue Feb 11, 2024 · 1 comment · Fixed by #243
Closed

Code for formats where there is an x.y is currently not creating all combinations within xpt_validate() function. #234

sophie-gem opened this issue Feb 11, 2024 · 1 comment · Fixed by #243
Assignees
@elimillera @sophie-gem
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@sophie-gem
Copy link
Collaborator

Within the utility function xpt_validate() in utils-xportr.r - there is a section where a vector of expected_formats is created. Whilst looking through this for ideas for #164, I noticed that lines 259, 262, 267, 276 are not creating all combinations of the vectors of numbers, that is the second vector is instead being recycled to the length of the first. For example, take line 259:

image

This gives the output:

image

whereas I think it should say, "E8601TM8.0" "E8601TM8.1" "E8601TM8.2" ... etc.

Think this can be resolved by using the sort() and rep() functions. Something like, paste("E8601TM", 8:15, ".", sort(rep(0:6, 8)), sep = "").
@sophie-gem sophie-gem added bug Something isn't working good first issue Good for newcomers labels Feb 11, 2024
@bms63 bms63 mentioned this issue Feb 20, 2024
Merged
15 tasks
@sophie-gem sophie-gem linked a pull request Feb 25, 2024 that will close this issue
Closes #164 Closes #228 Closes #234 xportr format messaging #243
Merged
15 tasks
sophie-gem added a commit that referenced this issue Feb 25, 2024
@sophie-gem
#164 #228 #234 - Selected the accurate expected_formats from `xport…
d45e6cf 
…r_format()` and added to the `xportr-utils.R` script.
@sophie-gem sophie-gem self-assigned this Feb 25, 2024
@sophie-gem
Copy link
Collaborator Author

@elimillera - we decided to just correct this issue as part of the xportr_format() issue.

bms63 added a commit that referenced this issue Feb 29, 2024
@sophie-gem @bms63
Closes #164 Closes #228 Closes #234 xportr format messaging (#243)
05177c6 
* #164 - add checks and messaging to `xportr_format`.

* #164 - Changing `cli` messaging for `xportr_logger` to ease testing. Begin to add in tests for these checks.

* #164 Finish adding tests for format checks. Add user documentation for these checks.

* #164 - styler and lintr alterations.

* #164 - Changes due to devtools::check() and devtools::document().

* #164 - Update function reference documentation according to appearance on built site.

* #164 - Add changes to `NEWS.md`.

* #164 - updates due to spelling, lintr, styler CI/CD errors.

* chore: #164 removed CDSIC from spelling

* Update NEWS.md

Update NEWS.md according to changes requested.

Co-authored-by: Ben Straub <[email protected]>

* #164 #228 #234 - Selected the accurate `expected_formats` from `xportr_format()` and added to the `xportr-utils.R` script.

* #164 - Tests added to check case-sensitivity as requested.

* #164 - Made varying review changes requested and updated tests.

* Update .lintr

---------

Co-authored-by: Ben Straub <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@elimillera elimillera

@sophie-gem sophie-gem

Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Closes #164 Closes #228 Closes #234 xportr format messaging atorus-research/xportr
2 participants
@elimillera @sophie-gem