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

Broken example in visualize_pathways_year_level2() + level1 #137

Merged
merged 13 commits into from
Jul 17, 2024

Conversation

PietrH
Copy link
Member

@PietrH PietrH commented Jul 8, 2024

During review I noticed that an example in visualize_pathways_year_level2() wouldn't work unless dplyr was already loaded. I added some extra namespace calls to dplyr.

Then, when running the unit tests, these failed because they were using dplyr functions that are not in the NAMESPACE without the dplyr:: in front of them.

I also switched out some calls to tidyselect to dplyr because this was shorter and this might help removing a direct dependency on tidyselect, altough you'll still be indirectly dependent on it though dplyr of course. I'm not sure if this causes the package to need a certain minimum version of dplyr.

EDIT: just saw the same for visualize_pathways_year_level1()

@PietrH PietrH requested a review from damianooldoni July 8, 2024 13:31
@PietrH PietrH changed the title Broken example in visualize_pathways_year_level2() Broken example in visualize_pathways_year_level2() + level1 Jul 8, 2024
Copy link
Collaborator

@damianooldoni damianooldoni left a comment

Choose a reason for hiding this comment

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

Thanks. I have also found some other missing dplyr:: 😄
I have also solved some check notes and improved a broken example which was in a \dontrun{ }. I removed the \dontrun{ } as well.
Another example on the other hand, has been put wrapped in a \dontrun{ } as it requires internet connection.

@damianooldoni damianooldoni merged commit 5e293f2 into main Jul 17, 2024
6 checks passed
@damianooldoni damianooldoni deleted the add-missing-dplyr-namespace branch July 17, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants