jmeskas / flowCut

Precise and Accurate Automated Removal of Outlier Events and Flagging of Files Based on Time Versus Fluorescence Analysis.
8 stars 9 forks source link

the flowDensity dependency cannot be installed anymore due to removal of a dependency from CRAN - could we integrate the dependencies directly into flowCut? #10

Open FMKerckhof opened 1 year ago

FMKerckhof commented 1 year ago

Hi,

Currently flowCut is importing deGate, plotDens and getPeaks from the flowDensity package. Sadly, due to the removal of RFOC from CRAN on 2023-08-25 it is not possible to install flowDensity anymore with just a call to install.packages() a hacky and unsustainable workaround is to install a previous version of RFOC from the CRAN archives (e.g. with devtools::install_version) and hope for the best.

I think this issue will come up again since r-spatial is phasing out multiple dependencies of flowDensity such as rgeos which becomes EOL by oct. 2023 (https://r-spatial.org/r/2023/05/15/evolution4.html).

Sadly, I do not have the insights to port flowDensity myself to not rely anymore on rgeos, RFOC, ... However, I have done a cursory exploration of deGate and the associated helper functions, also for plotDens(which internally relies on a flowViz function, but flowViz doesn't have any of the r-spatial imports) there is no issue, nor for the getPeaks.

I was wondering if it would be sensible to add these functions directly to flowCut (while respecting the reference to the original authors and the license), to reduce this specific dependency - flowDensity has a lot more complex functionality which relies on r-spatial packages which are not needed here.

What is your take on this @jmeskas ?

Kind regards,

FM


Relevant issues in flowDensity:

https://github.com/mehrnoushmalek/flowDensity/issues/9 https://github.com/mehrnoushmalek/flowDensity/issues/6

jmeskas commented 1 year ago

Hi @FMKerckhof ,

Thank you for bringing this to my attention. I agree that removing flowDensity from the dependency would be a good way to go. And I agree that we could copy the deGate and depending functions from the flowDensity package. In regards to the plotDens function perhaps leaving it rely on flowViz is acceptable, but we have also removed the plotDens dependency locally by using scattermore. From our experience scattermore creates essentially identical figures but in a much shorter time. Do you see any issue with us using scattermore? If not, then I can see if I can update flowCut before Oct 2023 to not require flowDensity any longer. Let me know what you think.

Thank you, Justin

jmeskas commented 1 year ago

Hi @FMKerckhof ,

I now see you sent a PR. I will look into this when I can and get back to you. Thank you so much for taking the time to make a PR.

Justin

FMKerckhof commented 1 year ago

Hi @jmeskas ,

The PR was intended to make flowCut installable out-of-the box - I think your idea to have plotDens using scattermore is a very interesting one (especially given the performance gain). I don't know if you are involved with the ggcyto project (https://github.com/RGLab/ggcyto) but even there sometimes it is challenging for high-density data with many overlays - so a new approach to plotting scatterplots is generally an interesting idea 😺

Kind regards,

FM

hrj21 commented 4 months ago

Hello, I believe this is related (but I can create a new issue if not), but despite reporting a successful installation through BiocManager::install("flowCut"), when I call library(flowCut) I get the error

Error: package or namespace load failed for ‘flowCut’ in loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]):
 there is no package called ‘rgeos’

which is due to the retirement of the rgeos package. Is there a current workaround for this?

Best wishes Hefin

FMKerckhof commented 4 months ago

Hi @hrj21 I do not have the ability to merge PR's (#11 ) myself, since I am not a repository owner - I think the "quick" fix I suggested is exactly that - it doesn't address a larger overhaul that @jmeskas is thinking about. However, if a quick fix is good enough for you you could install from our fork (I am doing so myself currently) - remotes::install_github(kytos-be/flowcut).

jmeskas commented 3 months ago

Hi @FMKerckhof and @hrj21,

Sorry for taking so long to reply. @FMKerckhof 's branch should solve your issue, and I am assuming that is what you are using. I just wanted to point out that flowDensity was updated 7-10 months ago and no longer requires the following packages:

Thank you, Justin