search

Bioconductor / BBS

The Bioconductor Build System
9 stars 11 forks source link

Remove brew install python from macosx documentation #384

Closed jwokaty closed 9 months ago

jwokaty commented 10 months ago

Close #382.

I removed the install python section from the macosx documentation and added a note that brewed installed pythons can take precedence over the default system Python.

@hpages The reason we needed this previously was for tensorflow. It's now available on all macs; however, it's not clear to me what package needed this. At first I thought it was deePINCS since it imports CRAN tensorflow, which lists tensorflow as a system requirement; however, it seemed to be passing on lconway, which doesn't have tensorflow: https://bioconductor.org/checkResults/3.18/bioc-LATEST/DeepPINCS/. Do you remember the Bioc package?

hpages commented 10 months ago

Glad that all the crazy Python dance on Mac is no longer needed.

At first I thought it was deePINCS since it imports CRAN tensorflow.

It's DeepPINCS.

Keep in mind that a package can import (or depend on) the tensorflow package and still pass R CMD check without the tensorflow Python module being available. That actually seems to be the case for a couple other packages (e.g. GenProSeq or VAExprs), not just DeepPINCS.

The tensorflow package itself will pass R CMD check without the tensorflow Python module being available. It achieves this by simply avoiding to have any real examples in the vignette, man pages, or unit tests, so we can't really blame Bioconductor packages for following that kind of practice :disappointed:

I do see that DeepPINCS is indeed marked in Prepare-Ubuntu-22.04-HOWTO.md as a package that requires the tensorflow Python module. I don't know if this is still true on Linux (this would need to be re-evaluated), but as far as Mac is concerned, that doesn't seem to be the case anymore (if it ever was).

Do you remember the Bioc package?

I don't :wink:

But if the absence of the tensorflow Python module on lconway doesn't cause any R CMD check error, then that means that no package needs it. You could check this for example by doing grep tensorflow *.checksrc-out.txt in ~biocbuild/public_html/BBS/3.18/bioc/products-in/lconway/checksrc on nebbiolo2 when the builds are done.

Thanks for updating the doc.

jwokaty commented 9 months ago

Thanks for your comments. Grepping for tensorflow in lconway's checksrc returned no results, so I guess it isn't needed on the macs.