maximilianh / cellBrowser

main repo: https://github.com/ucscGenomeBrowser/cellBrowser/ - Python pipeline and Javascript scatter plot library for single-cell datasets, http://cellbrowser.rtfd.org
https://github.com/ucscGenomeBrowser/cellBrowser/
GNU General Public License v3.0
104 stars 40 forks source link

Example build fails on MacOS with python2 #40

Closed ivirshup closed 5 years ago

ivirshup commented 5 years ago

Similar to #35, running the example build command throws an error. Here's the traceback:

Traceback (most recent call last):
  File "../../src/cbBuild", line 9, in <module>
    cellbrowser.convertAndCopyCli()
  File "../../src/cbPyLib/cellbrowser.py", line 2393, in convertAndCopyCli
    convertAndCopy(confFnames, outDir, port)
  File "../../src/cbPyLib/cellbrowser.py", line 2364, in convertAndCopy
    convertDataset(inConf, outConf, datasetDir)
  File "../../src/cbPyLib/cellbrowser.py", line 2131, in convertDataset
    convertExprMatrix(inConf, outMatrixFname, outConf, sampleNames, geneToSym, datasetDir, needFilterMatrix)
  File "../../src/cbPyLib/cellbrowser.py", line 1827, in convertExprMatrix
    copyMatrixTrim(matrixFname, outMatrixFname, metaSampleNames, needFilterMatrix)
  File "../../src/cbPyLib/cellbrowser.py", line 1430, in copyMatrixTrim
    matIter.open(inFname)
  File "../../src/cbPyLib/cellbrowser.py", line 663, in open
    encoding='utf-8',
TypeError: __init__() got an unexpected keyword argument 'encoding'

From what I can tell, this is caused by having the shebang of cbBuild call for python2 to be used and the usage of encoding as an argument to subprocess.Popen, as introduced by #36. In my python v2.7.15 installation, subprocess.Popen does not have an encoding argument:

>>> inspect.getargs(subprocess.Popen.__init__.__code__)
Arguments(args=['self', 'args', 'bufsize', 'executable', 'stdin', 'stdout', 'stderr', 'preexec_fn', 'close_fds', 'shell', 'cwd', 'env', 'universal_newlines', 'startupinfo', 'creationflags'], varargs=None, keywords=None)

I can get around this error by using:

$ python3 ../../src/cbBuild -o ~/public_html/cb/ -p 8888
maximilianh commented 5 years ago

Yes, this was caused by yesterday's PR from someone else. I had tested it only on python3. I'll change the python version in the shebang and also this particular line.

maximilianh commented 5 years ago

This brings up this whole annoying python3 unicode issue. I try to avoid unicode strings as they're slower, but here it looks like I have to use it. I've tested it now and it seems to work, but may look one day into the unnecessary use of utf8 here.

On Wed, Oct 3, 2018 at 12:15 PM Maximilian Haeussler maximilianh@gmail.com wrote:

Yes, this was caused by yesterday's PR from someone else. I had tested it only on python3. I'll change the python version in the shebang and also this particular line.

maximilianh commented 5 years ago

Thanks for bringing this up!

On Wed, Oct 3, 2018 at 1:23 PM Maximilian Haeussler maximilianh@gmail.com wrote:

This brings up this whole annoying python3 unicode issue. I try to avoid unicode strings as they're slower, but here it looks like I have to use it. I've tested it now and it seems to work, but may look one day into the unnecessary use of utf8 here.

On Wed, Oct 3, 2018 at 12:15 PM Maximilian Haeussler < maximilianh@gmail.com> wrote:

Yes, this was caused by yesterday's PR from someone else. I had tested it only on python3. I'll change the python version in the shebang and also this particular line.

maximilianh commented 5 years ago

Also, I cannot see any shebang lines anymore, even for yesterday's master branch, that specify python2. I wonder if you could do another git pull. In the master branch, it should all be just "python" now.

On Wed, Oct 3, 2018 at 1:23 PM Maximilian Haeussler maximilianh@gmail.com wrote:

Thanks for bringing this up!

On Wed, Oct 3, 2018 at 1:23 PM Maximilian Haeussler maximilianh@gmail.com wrote:

This brings up this whole annoying python3 unicode issue. I try to avoid unicode strings as they're slower, but here it looks like I have to use it. I've tested it now and it seems to work, but may look one day into the unnecessary use of utf8 here.

On Wed, Oct 3, 2018 at 12:15 PM Maximilian Haeussler < maximilianh@gmail.com> wrote:

Yes, this was caused by yesterday's PR from someone else. I had tested it only on python3. I'll change the python version in the shebang and also this particular line.

ivirshup commented 5 years ago

You're totally right, I must have been looking at cellbrowser.py on ace4d882f9. In general though, homebrew sets a symlink for python->python3, so that's probably why python2 was being used for me.

maximilianh commented 5 years ago

I certainly strive to make at least cbBuild work with both python2 and python3, so thanks for opening this ticket. I'm closing it now as the issue is hopefully solved. Let me know if you run into any other issues.

Good to know about homebrew having python3 as the default. ArchLinux is the same, most other linux distros will keep python2 as the default for a while.

On Thu, Oct 4, 2018 at 2:18 AM Isaac Virshup notifications@github.com wrote:

You're totally right, I must have been looking at cellbrowser.py on ace4d88 https://github.com/maximilianh/cellBrowser/commit/ace4d882f99bb6034219bc75f9230141741d3b3e. In general though, homebrew sets a symlink for python->python3, so that's probably why python2 was being used for me.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/maximilianh/cellBrowser/issues/40#issuecomment-426844845, or mute the thread https://github.com/notifications/unsubscribe-auth/AAS-TSsjCpLULTVOXBmdhQQWwVK1Wmspks5uhVPLgaJpZM4XFYQX .