mjwestgate / revtools

Tools to support research synthesis in R
https://revtools.net
48 stars 26 forks source link

issues screen_abstracts after screen_titles. #20

Closed MaraAlexeev closed 5 years ago

MaraAlexeev commented 5 years ago

Hello,

I am actually fairly new to R, and this is my first bug report really, so I apologize if it isn't up to standards. Thanks for revtools, it really seems fantastic.

My issue and work around is as below.

I first screened a batch of articles via screen_titles, then tried to screen the non-excluded papers from that in screen_abstracts() but no abstracts were showing up even though all but one article had an abstract in my df. I did a little tinkering and got it to show the abstracts. Here's my modified code.

Thanks!

#naming object from my search results
example_corpus <- readRDS("/cloud/project/storedAnalysis/documentCorpus.RDS")

#loaded example_corpus into screen_titles() in app and screened all of them. 

#read in screened corpus to object
screened_corpus <- read.csv("screened_corpus.csv")

#wanted to just look at non-excluded abstracts
to_screen_abstracts <- filter(screened_corpus, selected != "excluded")

#then I tried to screen_abstracts(to_screen_abstracts), but only titles and no abstracts would show up in the shiny app

#My workaround that worked! 
#I think the first step to remove some columns was probably not necessary and that main issue may be the abstract vs Abstract issue. 

ready_to_screen_abstracts <- select(to_screen_abstracts, -c(selected, order, notes, page))
colnames(ready_to_screen_abstracts)[colnames(ready_to_screen_abstracts)=="Abstract"] <- "abstract"
mjwestgate commented 5 years ago

Hi Mara - thanks for getting in touch! It's great to hear that you're enjoying revtools, and sorry that you've had problems.

This is a funny problem; it's a bit difficult to see what's happening here without a little more information. Certainly you're right that screen_abstracts looks for a column named 'abstract' and only displays abstracts from that column, so your fix is correct. This could easily be avoided if screen_abstracts forced column names to lower case, but a more general solution would be to allow user-specified column names in screen_abstracts (screen_topics already does this, for example). I can try and find some time to build that feature in if you think it would be helpful.

My question is why was there a column labelled 'Abstracts' in the first place? I.e. did you initially import the data in documentCorpus.RDS into R using revtools, or some other method? If you used revtools then read_bibiliography is also faulty and needs updating, but otherwise I'll just fix screen_abstracts :)

MaraAlexeev commented 5 years ago

I used a packaged called Adjutant to generate the corpus which had made an .RDS of the citations. Then I used screen_titles directly on that data, and that worked fine, and I saved screened data to a new object. Then I tried to use screen_abstracts on that object and that's when I couldn't see any of the abstracts though I knew they were included in the data. Going back now and reading the documentation for read_bibliography I think I just got lucky that the Adjutant package output the data in a way that screen_titles could read but that screen_abstracts had an issue with.

mjwestgate commented 5 years ago

Ahh I see! I haven't seen Adjutant before (it looks good!), but that makes sense now.

I think what I'll do for now is update the documentation for the screening apps so that users know how the column detection works; and add in a colnames(x) <- tolower(colnames(x)) stage to account for this specific problem. That way people won't have to guess at the naming convention like you had to! If anyone has a more complex naming structure to their documents they'll have to revert to read_bibliography or manually rename columns.

Let me know if there are other changes you need; otherwise I'll update this again when the changes are on GitHub

mjwestgate commented 5 years ago

This bug appears fixed as of this commit

MaraAlexeev commented 5 years ago

It works for me! Thank you!

mjwestgate commented 5 years ago

Excellent! I'll close this now, but feel free to re-open or open a new one if you find more bugs. BTW I've just sent v0.4.0 to CRAN, so this fix will be in the stable version soon.

MaraAlexeev commented 5 years ago

Will do!

On Wed, Jul 10, 2019, 18:46 Martin Westgate notifications@github.com wrote:

Excellent! I'll close this now, but feel free to re-open or open a new one if you find more bugs. BTW I've just sent v0.4.0 to CRAN, so this fix will be in the stable version soon.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mjwestgate/revtools/issues/20?email_source=notifications&email_token=AJOV6YNZ3SXL7OP4VHIRTV3P6ZRDBA5CNFSM4HWHSE62YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZU65PQ#issuecomment-510258878, or mute the thread https://github.com/notifications/unsubscribe-auth/AJOV6YNMA3CLH2N7MOE2T5LP6ZRDBANCNFSM4HWHSE6Q .