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

Update: Closes #98 removing symbols that are no longer illegal #119

Merged
merged 6 commits into from
May 24, 2023

Conversation

sadchla-codes
Copy link
Collaborator

No description provided.

@sadchla-codes sadchla-codes marked this pull request as draft May 11, 2023 15:47
@sadchla-codes sadchla-codes added this to the xportr 0.3.0 milestone May 11, 2023
@bms63 bms63 requested a review from cpiraux May 18, 2023 15:50
@bms63
Copy link
Collaborator

bms63 commented May 19, 2023

@sadchla-codes can you fix the failing tests? We should switch the <> to another illegal character to keep test coverage.

@cpiraux can take over this issue if you need?

@sadchla-codes sadchla-codes marked this pull request as ready for review May 23, 2023 15:08
expect_equal(
xpt_validate(df),
"Label 'A=foo%bar' cannot contain any non-ASCII, symbol or special characters."
"Label 'A=foo&#191;bar' cannot contain any non-ASCII, symbol or special characters."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

% is an ASCII character, you could use @

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpiraux I don't know why @ escaped my mind... I know the PR is approved, and I used ç, would you prefer i switched it to @ instead? or you comfortable with me merging it to devel?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ç is also fine. You can merge the branch to devel :)

@sadchla-codes sadchla-codes merged commit dc7fb3b into devel May 24, 2023
@sadchla-codes sadchla-codes deleted the 98_change_illegal_chr_check branch May 24, 2023 12:08
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.

< and > symbols are no longer considered illegal characters in variable and dataset labels
3 participants