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

Fix #61 #62

Merged
merged 1 commit into from
Aug 12, 2022
Merged

Fix #61 #62

merged 1 commit into from
Aug 12, 2022

Conversation

chainsawriot
Copy link
Contributor

The fix of #57 makes an assumption that all items contain a
description tag. But it is not always the case and for those item
map(res_entry, "description", .default = def) will be NA (the
def) and the equality operator in the subsequent predicate function dies.

I've included a simple test.

The fix of RobertMyles#57 makes an assumption that all items contain a
description tag. But it is not always the case and for those item
`map(res_entry, "description", .default = def)` will be `NA` (the
`def`) and the equality operator in the subsequent predicate function dies.

I've included a simple test.
@chainsawriot
Copy link
Contributor Author

BTW, I think all

map(blah) %>% unlist

can be replaced by

purrr::map_chr(blah)

don't you think?

@RobertMyles
Copy link
Owner

Hey @chainsawriot , thank you for the PR, much appreciated! I'm really struggling to find time to revisit this problem. As for map(blah) %>% unlist can be replaced by purrr::map_chr(blah), you're probably right, but I think I had something like that previously and I ran into a problem somewhere or other. Let me think about it again.

I'll review your code and I'm running some tests on it and after I'll approve if all goes well.

@lgnbhl
Copy link

lgnbhl commented Aug 7, 2022

Hi @chainsawriot, thank you very much for the fix!

@RobertMyles RobertMyles merged commit f37726a into RobertMyles:master Aug 12, 2022
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.

None yet

3 participants