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

Use xml_find_int() over xml_find_num() #2452

Open
MichaelChirico opened this issue Dec 15, 2023 · 1 comment
Open

Use xml_find_int() over xml_find_num() #2452

MichaelChirico opened this issue Dec 15, 2023 · 1 comment
Labels
internals Issues related to inner workings of lintr, i.e., not user-visible

Comments

@MichaelChirico
Copy link
Collaborator

Recent (Dec. 2023) xml2 v1.3.6 exports xml_find_int() which basically does as.integer(xml_find_num(.)).

#2439 found that all of our internal usages work just like that -- convert xml_find_num() output to integer. Therefore we should switch everything to the new function.

Tried this in #2439 directly but it caused some issues. Anyway, it might be best to let the hot-off-the-presses {xml2} release simmer a bit before we include the functionality, so I'll leave this to the next release.

@IndrajeetPatil IndrajeetPatil added the internals Issues related to inner workings of lintr, i.e., not user-visible label Dec 16, 2023
@MichaelChirico
Copy link
Collaborator Author

The core issue is that xml_find_int() needs to be more robust to missing input:

r-lib/xml2#435

This is an illustrative test:

xp_invalid <- "number(./DOES-NOT-EXIST/@col1)"

With xml_find_num(), we get NaN back and handle it. But xml_find_int() throws an error when it gets NaN. Need to handle this upstream first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internals Issues related to inner workings of lintr, i.e., not user-visible
Projects
None yet
Development

No branches or pull requests

2 participants