search

dirkschumacher / rcbc

COIN-OR branch and cut (CBC) bindings for R
https://dirkschumacher.github.io/rcbc/
Other
19 stars 5 forks source link

Documentation updates #41

Closed jeffreyhanson closed 3 years ago

jeffreyhanson commented 3 years ago

This PR aims to help improve documentation. Specifically, it adds additional details and examples to the function documentation and adds a vignette (per #12). As discussed over email, I have also updated the DESCRIPTION to list myself as a contributor. To try and make this PR as easy as possible to review, I have made multiple commits related to specific changes. Is this actually helpful? If not, please let me know and I can squash all changes into a single commit.

jeffreyhanson commented 3 years ago

I wasn't sure exactly what information needed to be in the vignette, but hopefully it provides a useful starting point for new users? Let me know if there's additional topics/functionality that should be covered? Also, please feel free to edit/change/update any of the documentation in this PR if anything's not clear (or in case of typos)?

jeffreyhanson commented 3 years ago

Also, since this PR only covers documentation-related stuff, the Windows CI checks are still failing (per #34).

dirkschumacher commented 3 years ago

Thank you so much 🙏. I will try to take a deeper look before merging.

jeffreyhanson commented 3 years ago

Ah - now we've got merge conflict issues due to the automated package website rebuild. Sorry about that - I'll take care of this now.

jeffreyhanson commented 3 years ago

After playing around some more with the rcbc, I realized that the CBC parameters should be in lower case to actually work. Currently, the documentation in this PR incorrectly provides the parameters in upper case. I'll update the documentation to address this mistake.

jeffreyhanson commented 3 years ago

Ah - I'm sorry, I just realized this PR has merge conflict issues. I'll try and address them now.

jeffreyhanson commented 3 years ago

I'm sorry, this PR has a lot of unnecessary commits. Is that an issue? Please let me know if you would like me to squash all this into a single commit, or open up a new PR (based on a new branch) with just the relevant changes copy-pasted from this branch?

dirkschumacher commented 3 years ago

Really great stuff. Thanks for writing that and also adding some literature references.

jeffreyhanson commented 3 years ago

No worries - I'm happy to help! Thanks so much for making the time to look at this.

jeffreyhanson commented 3 years ago

Ah - I've introduced another merge conflict. I'll fix that now.

jeffreyhanson commented 3 years ago

Ok - I think I've addressed all the issues with this PR. @dirkschumacher, please let you know if there's any further I changes I can make to improve it?

dirkschumacher commented 3 years ago

Great thanks. LGTM!

jeffreyhanson commented 3 years ago

Awesome - thanks! I'll work on updating the README now and adding citation information (e.g. so the users are told to cite the rcbc package and also CBC when using this package).