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

Documentation and error handling for 'tree.order' option #41

Merged
merged 3 commits into from
Aug 3, 2023

Conversation

befriendabacterium
Copy link
Contributor

-Added @param with documentation
-Added error handler - if user specifies order of incorrect length, stops function and throws error

-Added @param with documentation
-Added error handler - if user specifies order of incorrect length, stops function and throws error
@befriendabacterium
Copy link
Contributor Author

Oh great it's failing everything now 😅 even worse than last time. Eyeballing the error messages it seems to be having trouble installing orchaRd 🤔

@daniel1noble
Copy link
Owner

Ha, happens. If you use devtools::check() and devtools::tests() you should be able to narraow the issues down

@befriendabacterium
Copy link
Contributor Author

Ha, happens. If you use devtools::check() and devtools::tests() you should be able to narraow the issues down

Hi Dan - sorry but I'm not really understanding. orchaRD is installed fine my end, it's just when I make the pull request on GitHub these issues arise. I am completely unsure how these checks are working but it seems to be something with the installation of orchaRd on the virtual machine the checks are running off, in my very naive understanding?

- Added 'tree.order' parameter to the function (forgot before, doh!)
-Minor correction for coping with tree.order=NULL
- Removed extra bracket on line 103 (flagged by checkout)
- Got ordering working properly by adding line to reorder factor levels for data_trim (as well as previous reordering of mod_table)
@befriendabacterium
Copy link
Contributor Author

Alright with help of a friend @padpadpadpad think have figured out the error and implemented some other corrections to actually get it working...famous last words 🤞

@padpadpadpad
Copy link

You need to re-run devtools::document() to make sure the help page has your new parameter in.

@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Patch coverage: 40.00% and project coverage change: -0.17% ⚠️

Comparison is base (e837246) 60.48% compared to head (49f4b4e) 60.32%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
- Coverage   60.48%   60.32%   -0.17%     
==========================================
  Files           9        9              
  Lines         615      620       +5     
==========================================
+ Hits          372      374       +2     
- Misses        243      246       +3     
Files Changed Coverage Δ
R/orchard_plot.R 71.77% <40.00%> (-1.34%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@befriendabacterium
Copy link
Contributor Author

befriendabacterium commented Aug 2, 2023

seems to be running ok now just codecov couldn't check some lines I added but I think OK to merge pending one last brief check from you?

@daniel1noble
Copy link
Owner

@befriendabacterium This is looking good on my end so I'll merge. Thanks for adding that, and thanks @padpadpadpad for clarifying to Matt on what to do.

@daniel1noble daniel1noble merged commit 317ee6c into daniel1noble:main Aug 3, 2023
4 of 6 checks passed
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