brittanyblouin / ANCRTAdjust

An R package to adjust routine HIV testing data from antenatal care to reduce bias in estimating HIV prevalence trends
MIT License
2 stars 3 forks source link

Use of plyr #13

Open seabbs opened 5 years ago

seabbs commented 5 years ago

It was my understanding that plyr had been retired (in favour of dplyr) with support really only there for legacy applications.  

Could you give me some justification for using it? Alternatively switching out to dplyr would be great.

For this #openjournals/joss-reviews/issues/1740 review

m-maheu-giroux commented 4 years ago

Our understanding was that dplyr cannot accommodate user defined functions at this time. Hence, we went with plyr.

seabbs commented 4 years ago

mutate , purrr::map and then unnest supports this. Happy to walk through in more detail if needed or explore other options.

Alternatively point me at the code which requires plyr and I will give refactoring a shot.

(Not sure what the etiquette here is but I would find it helpful if you left the review issues for us to close - again really helps keep track of progress)

m-maheu-giroux commented 4 years ago

I initially closed them because it was easier for me to go through your comments... but the point is to facilitate your work - not mine. I have left the other issues for you to close.

seabbs commented 4 years ago

No problem at all - thanks for doing that.

Did you have any thoughts on switching to dplyr. I had another look at your code base and it does all look pretty do-able.

@ellessenne do you agree with me on this or is the current implementation okay?

ellessenne commented 4 years ago

I would suggest not using deprecated packages - they may suddenly stop working with new releases of R. If it is not too hard to move on from plyr, I would recommend doing so.

On a side note, plyr is imported twice in the DESCRIPTION file - which causes the following issue when running R CMD check:

❯ checking DESCRIPTION meta-information ... NOTE
  Package listed in more than one of Depends, Imports, Suggests, Enhances:
    ‘plyr’
  A package should be listed in only one of these fields.