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

E8601TZ formats within xpt_validate() function contain spaces due to missing sep =. #228

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

E8601TZ formats within xpt_validate() function contain spaces due to missing sep =. #228

sophie-gem opened this issue Jan 30, 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 261 & 262,

image

are missing the sep = "" operator meaning that the formats unintentionally contain spaces:

image

Think this will need updating otherwise the formats will not get correctly matched.
@sophie-gem sophie-gem added bug Something isn't working good first issue Good for newcomers labels Jan 30, 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