mcguinlu / robvis

A package to quickly visualise risk-of-bias assessment results
https://mcguinlu.github.io/robvis/
Other
58 stars 22 forks source link

Adding QUIPS tool #86

Closed mcguinlu closed 3 years ago

mcguinlu commented 4 years ago

Hi @tdbennett,

I noticed that you had forked this repo and added a new tool template to robvis. I'll be working towards a new CRAN release of robvis next week and hoped to pull in your changes as part of a range of update to the package. Adding QUIPS is a great idea and will probably benefit other users, but I've never heard about/used it before!

There are a few things I wanted to follow-up on before pulling in the changes:

Hope you are interested in working on this, as the QUIPS tool would be a great addition to robvis. In terms of recognition for your work, I'm planning to add you to a new "Contributors" section in the README (see here for an example of what this will look like!).

Luke

tdbennett commented 4 years ago

Hi Luke-

Thanks very much for your request. It has been on my list to clean up the fork and submit a pull request, but obviously I haven't gotten there yet:). Answers to your questions:

When are you intending to submit? I'm happy to do this work but might not be able to do it this week.

Best wishes,

-Tell

mcguinlu commented 4 years ago

Great - thanks for the extra info!

I first attempted to extend the "Generic" tool option to accommodate QUIPS, but ultimately chose to add another tool option. I believe that I've backed out all of the changes I made to "Generic/ROB1" but will want to double-check before merging with the main repo.

Perfect - will do!

I'll have to take a look at the example datasets. I've used the tool for several manuscripts in a compendium to be submitted for publication soon and will appropriately cite your work. I can't release the data yet, but maybe I can generate a synthetic version.

All the example datasets provided by robvis are synthetic, so that would be fine!

Per your example elsewhere in the package documentation, I just created a weight column in my analysis script (in another, private, repo) and set it to 1 for all rows in order to generate the bar plot. That works just fine.

Was this using the "Generic" template (i.e. rob_summary(tool="Generic"))? If we are adding a tool option/formal template for QUIPS to the traffic light function (i.e. rob_traffic_light(tool = "QUIPS")) I would like to have a corresponding template for the summary barplot function (i.e. rob_summary(tool = "QUIPS")), if that makes sense?

I can take a look at testing. I have some limited experience writing tests.

Not to worry - I can write some tests for it.

When are you intending to submit? I'm happy to do this work but might not be able to do it this week.

I plan to do a pretty big overhaul (and address a backlog of issues) in the coming two weeks, with a plan to submit to CRAN at the end of August. So no huge rush on this work, or pressure to do it at all - I fully appreciate that it is summer and (for me at least) all work on this package is voluntary.

But thanks again for your efforts to date!

mcguinlu commented 3 years ago

Hi @tdbennett,

Just to flag that, as of #107, the QUIPS tool is officially supported by robvis 🎉

Due to an overhaul in the code base, I incorporated your code by hand rather than merging in this request, but just wanted to say thanks for your interest/work on this, and please let me know if you spot any issues with the way I have implemented the QUIPS tool!

Luke

tdbennett commented 3 years ago

Thanks Luke!

Exciting new functionality. I’ll test out the QUIPS implementation in my next round of work on that project, time TBD.

Best wishes

On Nov 10, 2020, at 11:12 AM, Luke McGuinness notifications@github.com wrote:

Hi @tdbennett https://github.com/tdbennett,

Just to flag that, as of #107 https://github.com/mcguinlu/robvis/pull/107, the QUIPS tool is officially supported by robvis 🎉

Due to an overhaul in the code base, I incorporated your code by hand rather than merging in this request, but just wanted to say thanks for your interest/work on this, and please let me know if you spot any issues with the way I have implemented the QUIPS tool!

Luke

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mcguinlu/robvis/pull/86#issuecomment-724875474, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3BHSZDNJRWEAE57WRN2UTSPF7ATANCNFSM4PKJCWWA.