KarchinLab / open-cravat

A modular annotation tool for genomic variants
MIT License
113 stars 27 forks source link

Bug: Merge SQLite fails with python/3.6 #113

Closed skchronicles closed 2 years ago

skchronicles commented 2 years ago

Hello there,

I found a bug in the oc util mergesqlite subcommand.

It seems to fail when OpenCRAVAT is installed with python/3.6. I haven't tested python/3.7 but it seems to work fine in python/3.8.

Description

The oc util mergesqlite command fail with a TypeError in python/3.6.

Here is an example: image

I looked into this more, and it appears that Path returns a path object in python/3.6:

image

And this causes the sqlite.connect() method to fail: image

Solution

The actual fix is pretty stright forward. You could just loop through the paths within a list comprehension and type cast the path objects to strings. There are a few different ways to fix this issue but that seems the most straight forward method.

image
# insert after line 763 (before the if statement)
dbpaths = [str(db) for db in dbpaths]

I haven't had time to test out how this version of python effect the other sub commands. I have a feel some other commands may fail as well. With that being said, it maybe better to fix the problem at its actual cause (line 1113) when setting a type for this option:

image

You could just use a lambda function to type-cast the Path object to a string:

# update to line 1113
parser_mergesqlite.add_argument("path", nargs='+', help="Path to result database", type = lambda f: str(Path(f)))

Please let me know what you think.

Best, @skchronicles

skchronicles commented 2 years ago

Just as a heads up, I was also able to confirm this issue also exists within your Docker image for OpenCRAVAT. It may be worth using python/3.8 as your base image:

user@cn3106:/lscratch/39864423$ SINGULARITY_CACHEDIR=$PWD singularity pull -F docker://karchinlab/opencravat:latest
INFO:    Converting OCI blobs to SIF format
INFO:    Starting build...
Getting image source signatures
Copying blob 955615a668ce done  
Copying blob 2756ef5f69a5 done  
Copying blob 911ea9f2bd51 done  
Copying blob 27b0a22ee906 done  
Copying blob 8584d51a9262 done  
Copying blob db63833ee201 done  
Copying blob 670202beded7 done  
Copying blob 35e5210b09c9 done  
Copying blob b8adb6962ce2 done  
Copying blob 9a00cb98124d done  
Copying blob 3747a1cdb3fa done  
Copying blob 13fe569b962f done  
Copying blob edbd166503d1 done  
Copying config 57c0972c41 done  
Writing manifest to image destination
Storing signatures
2022/05/18 10:29:07  info unpack layer: sha256:955615a668ce169f8a1443fc6b6e6215f43fe0babfb4790712a2d3171f34d366
2022/05/18 10:29:09  info unpack layer: sha256:2756ef5f69a5190f4308619e0f446d95f5515eef4a814dbad0bcebbbbc7b25a8
2022/05/18 10:29:09  info unpack layer: sha256:911ea9f2bd51e53a455297e0631e18a72a86d7e2c8e1807176e80f991bde5d64
2022/05/18 10:29:10  info unpack layer: sha256:27b0a22ee906271a6ce9ddd1754fdd7d3b59078e0b57b6cc054c7ed7ac301587
2022/05/18 10:29:12  info unpack layer: sha256:8584d51a9262f9a3a436dea09ba40fa50f85802018f9bd299eee1bf538481077
2022/05/18 10:29:18  info unpack layer: sha256:db63833ee201770b3aa0d7071a972c3a03ae41d11d86061890e385fc2a41545a
2022/05/18 10:29:18  info unpack layer: sha256:670202beded7ea5f23ab5ea78f90eaf00e5f39cd978f6869c1545a725ab6946f
2022/05/18 10:29:19  info unpack layer: sha256:35e5210b09c9f1a52e3255faa346ca29df26389273a434d216abeea4ed01f7b1
2022/05/18 10:29:19  info unpack layer: sha256:b8adb6962ce28cc7a903503a1e1e1b4159aea23a4b32c9a47e8a2ec0c2b1e294
2022/05/18 10:29:19  info unpack layer: sha256:9a00cb98124dbc36c097ef6144b25eb374266d4733aaf3359b1ca29dafd52ef9
2022/05/18 10:29:20  info unpack layer: sha256:3747a1cdb3fa18f8938b8171d3c4dea0b7677db091ae48b488ecb4bc01052e13
2022/05/18 10:29:23  info unpack layer: sha256:13fe569b962f4603d95b43497cd11cc87a8a2edc77af190d6e34a7c9db408382
2022/05/18 10:29:23  info unpack layer: sha256:edbd166503d1d59c91ab1dc862bdf876149847e5594d22e3161e09964fa6c26a
INFO:    Creating SIF file...
user@cn3106:/lscratch/39864423$ ll
total 442M
drwxr-xr-x. 5 root   root 4.0K May 18 09:23 ..
-rw-rw----  1 user CCBR    0 May 18 09:37 2.sqlite
-rw-rw----  1 user CCBR    0 May 18 09:37 1.sqlite
-rw-rw----  1 user CCBR    0 May 18 09:37 3.sqlite
drwx------  8 user CCBR 4.0K May 18 10:28 cache
-rwxrwx---  1 user CCBR 442M May 18 10:29 opencravat_latest.sif
drwx------  3 user CCBR 4.0K May 18 10:29 .
user@cn3106:/lscratch/39864423$ singularity exec -B $PWD opencravat_latest.sif which oc
/usr/local/bin/oc
user@cn3106:/lscratch/39864423$ singularity exec -B $PWD opencravat_latest.sif oc util mergesqlite 1.sqlite 2.sqlite 3.sqlite -o merged.sqlite 
Traceback (most recent call last):
  File "/usr/local/bin/oc", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.6/site-packages/cravat/oc.py", line 220, in main
    args.func(args)
  File "/usr/local/lib/python3.6/site-packages/cravat/cravat_util.py", line 770, in mergesqlite
    conn = sqlite3.connect(dbpaths[0])
TypeError: argument 1 must be str, not PosixPath
skchronicles commented 2 years ago

I have pushed a new image to DockerHub that uses python/3.8 as the base image. Everything works fine with that version of python.

Here is the link to the new image: https://hub.docker.com/repository/docker/skchronicles/ncbr_opencravat

$ module load singularity
[+] Loading singularity  3.8.5-1  on cn0904 

$ SINGULARITY_CACHEDIR=$PWD singularity pull -F docker://skchronicles/ncbr_opencravat:latest
INFO:    Converting OCI blobs to SIF format
INFO:    Starting build...
Getting image source signatures
Copying blob 67e8aa6c8bbc done  
Copying blob 627e6c1e1055 done  
Copying blob 0670968926f6 done  
Copying blob 5a8b0e20be4b done  
Copying blob b0b10a3a2784 done  
Copying blob e16cd24209e8 done  
Copying blob 28cdae4e9d64 done  
Copying blob baae5790c39a done  
Copying blob 7c780354332c done  
Copying blob 758d5ac895a9 done  
Copying blob 4f4fb700ef54 done  
Copying blob 5d89363fa844 done  
Copying blob 39f9f8e89197 done  
Copying blob ca6a83e8c1ee done  
Copying blob 970d884f0bd2 done  
Copying blob 4f4fb700ef54 skipped: already exists  
Copying blob db3f6bd33e27 done  
Copying blob fb32577551be done  
Copying blob 4f4fb700ef54 skipped: already exists  
Copying config d7838d0861 done  
Writing manifest to image destination
Storing signatures
2022/05/18 12:10:49  info unpack layer: sha256:67e8aa6c8bbc76b1f2bccb3864b0887671833b8667dc1f6c965fcb0eac7e6402
2022/05/18 12:10:51  info unpack layer: sha256:627e6c1e105548ea4a08354eea581f137cf368d91aeb0ad47dcb706fca54fd8b
2022/05/18 12:10:51  info unpack layer: sha256:0670968926f6461e3135c82ba2c0ad3ebdedc0d0f41b18bda4a1e41104b8be8a
2022/05/18 12:10:52  info unpack layer: sha256:5a8b0e20be4b4a332bc3d90b9903a5f3c0664b440fd9f1d2a1db0d4b7e6e826b
2022/05/18 12:10:54  info unpack layer: sha256:b0b10a3a2784b06bfe0af1493ea2a0fa957ae5e0dc30fcfd1166d2558ba74e4d
2022/05/18 12:11:00  info unpack layer: sha256:e16cd24209e80fb99ff55cf53a1c2e4a062cffad4b24c6564ef6bb4fc9428827
2022/05/18 12:11:01  info unpack layer: sha256:28cdae4e9d64414426019c14798d34ed22ce4a3e58cb4ef907cf0aa5cccbe7a6
2022/05/18 12:11:01  info unpack layer: sha256:baae5790c39a348d46b7d6dd299f078e14f8bd47e1b9b16d47c72f097757088c
2022/05/18 12:11:01  info unpack layer: sha256:7c780354332cbc06b2b3c3c01882645db2b41d12d988bc2631d04581b306b127
2022/05/18 12:11:01  info unpack layer: sha256:758d5ac895a9fdb65050020a86905285f6c257ee35bbc00c4e6f00d181313ce2
2022/05/18 12:11:01  info unpack layer: sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1
2022/05/18 12:11:01  info unpack layer: sha256:5d89363fa844afc23a8b485e3201bc880d11b4f1ffd2f1211d17ecc635645d74
2022/05/18 12:11:01  info unpack layer: sha256:39f9f8e8919704b6ec9c1698808667ff4aa191aff0e9c2da5235d5f6b9a7c205
2022/05/18 12:11:02  info unpack layer: sha256:ca6a83e8c1eebbba3d1833f3e705704d03780b032a591e295d3044300aede39a
2022/05/18 12:11:02  info unpack layer: sha256:970d884f0bd2a49ba9b5e3312ef838ad3e5c7940b3fc368db57a227cabc548a1
2022/05/18 12:11:07  info unpack layer: sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1
2022/05/18 12:11:07  info unpack layer: sha256:db3f6bd33e27d2d51cd6b52ede72fc31ae017b06919b110a2411a884a40b6f3b
2022/05/18 12:11:07  info unpack layer: sha256:fb32577551be39be872939bd4afea9f414e32e02e465824701e3ac7ac3064d3f
2022/05/18 12:11:07  info unpack layer: sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1
INFO:    Creating SIF file...

$ ls -larth
total 726M
drwxr-xr-x. 7 root   root 4.0K May 18 12:09 ..
drwx------  8 user CCBR 4.0K May 18 12:10 cache
-rwxrwx---  1 user CCBR 455M May 18 12:11 ncbr_opencravat_latest.sif
-rw-r-----  1 user CCBR  69M May 18 12:20 cravat_chr21.fixed.sqlite
-rw-r-----  1 user CCBR  67M May 18 12:20 cravat_chr22.fixed.sqlite
drwx------  3 user CCBR 4.0K May 18 12:20 .

$ singularity exec -B $PWD ncbr_opencravat_latest.sif oc util mergesqlite cravat_chr21.fixed.sqlite cravat_chr22.fixed.sqlite -o merged.sqlite
Copying cravat_chr21.fixed.sqlite to merged.sqlite...
Merging cravat_chr22.fixed.sqlite...

$ echo $?
0

Everything works as expected with this version of python.

mryaninsilico commented 2 years ago

Skyler,

Thanks for taking the time to report this issue. Historically we have set 3.6 as the minimum Python version but I think it may be time to push that up to a newer version. 3.8 seems a reasonable choice as it has been out since 10/2019. We will talk it over and either correct the 3.6 compatibility issue or more likely change the OC packages to require >= 3.8.

Mike

From: Skyler Kuhn @.> Sent: Wednesday, May 18, 2022 12:31 PM To: KarchinLab/open-cravat @.> Cc: Subscribed @.***> Subject: Re: [KarchinLab/open-cravat] Bug: Merge SQLite fails with python/3.6 (Issue #113)

I have pushed a new image to DockerHub that uses python/3.8 as the base image. Everything works fine with that version of python.

Here is the link to the new image: https://hub.docker.com/repository/docker/skchronicles/ncbr_opencravat

$ module load singularity [+] Loading singularity 3.8.5-1 on cn0904

$ SINGULARITY_CACHEDIR=$PWD singularity pull -F docker://skchronicles/ncbr_opencravat:latest INFO: Converting OCI blobs to SIF format INFO: Starting build... Getting image source signatures Copying blob 67e8aa6c8bbc done
Copying blob 627e6c1e1055 done
Copying blob 0670968926f6 done
Copying blob 5a8b0e20be4b done
Copying blob b0b10a3a2784 done
Copying blob e16cd24209e8 done
Copying blob 28cdae4e9d64 done
Copying blob baae5790c39a done
Copying blob 7c780354332c done
Copying blob 758d5ac895a9 done
Copying blob 4f4fb700ef54 done
Copying blob 5d89363fa844 done
Copying blob 39f9f8e89197 done
Copying blob ca6a83e8c1ee done
Copying blob 970d884f0bd2 done
Copying blob 4f4fb700ef54 skipped: already exists
Copying blob db3f6bd33e27 done
Copying blob fb32577551be done
Copying blob 4f4fb700ef54 skipped: already exists
Copying config d7838d0861 done
Writing manifest to image destination Storing signatures 2022/05/18 12:10:49 info unpack layer: sha256:67e8aa6c8bbc76b1f2bccb3864b0887671833b8667dc1f6c965fcb0eac7e6402 2022/05/18 12:10:51 info unpack layer: sha256:627e6c1e105548ea4a08354eea581f137cf368d91aeb0ad47dcb706fca54fd8b 2022/05/18 12:10:51 info unpack layer: sha256:0670968926f6461e3135c82ba2c0ad3ebdedc0d0f41b18bda4a1e41104b8be8a 2022/05/18 12:10:52 info unpack layer: sha256:5a8b0e20be4b4a332bc3d90b9903a5f3c0664b440fd9f1d2a1db0d4b7e6e826b 2022/05/18 12:10:54 info unpack layer: sha256:b0b10a3a2784b06bfe0af1493ea2a0fa957ae5e0dc30fcfd1166d2558ba74e4d 2022/05/18 12:11:00 info unpack layer: sha256:e16cd24209e80fb99ff55cf53a1c2e4a062cffad4b24c6564ef6bb4fc9428827 2022/05/18 12:11:01 info unpack layer: sha256:28cdae4e9d64414426019c14798d34ed22ce4a3e58cb4ef907cf0aa5cccbe7a6 2022/05/18 12:11:01 info unpack layer: sha256:baae5790c39a348d46b7d6dd299f078e14f8bd47e1b9b16d47c72f097757088c 2022/05/18 12:11:01 info unpack layer: sha256:7c780354332cbc06b2b3c3c01882645db2b41d12d988bc2631d04581b306b127 2022/05/18 12:11:01 info unpack layer: sha256:758d5ac895a9fdb65050020a86905285f6c257ee35bbc00c4e6f00d181313ce2 2022/05/18 12:11:01 info unpack layer: sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1 2022/05/18 12:11:01 info unpack layer: sha256:5d89363fa844afc23a8b485e3201bc880d11b4f1ffd2f1211d17ecc635645d74 2022/05/18 12:11:01 info unpack layer: sha256:39f9f8e8919704b6ec9c1698808667ff4aa191aff0e9c2da5235d5f6b9a7c205 2022/05/18 12:11:02 info unpack layer: sha256:ca6a83e8c1eebbba3d1833f3e705704d03780b032a591e295d3044300aede39a 2022/05/18 12:11:02 info unpack layer: sha256:970d884f0bd2a49ba9b5e3312ef838ad3e5c7940b3fc368db57a227cabc548a1 2022/05/18 12:11:07 info unpack layer: sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1 2022/05/18 12:11:07 info unpack layer: sha256:db3f6bd33e27d2d51cd6b52ede72fc31ae017b06919b110a2411a884a40b6f3b 2022/05/18 12:11:07 info unpack layer: sha256:fb32577551be39be872939bd4afea9f414e32e02e465824701e3ac7ac3064d3f 2022/05/18 12:11:07 info unpack layer: sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1 INFO: Creating SIF file...

$ ls -larth total 726M drwxr-xr-x. 7 root root 4.0K May 18 12:09 .. drwx------ 8 user CCBR 4.0K May 18 12:10 cache -rwxrwx--- 1 user CCBR 455M May 18 12:11 ncbr_opencravat_latest.sif -rw-r----- 1 user CCBR 69M May 18 12:20 cravat_chr21.fixed.sqlite -rw-r----- 1 user CCBR 67M May 18 12:20 cravat_chr22.fixed.sqlite drwx------ 3 user CCBR 4.0K May 18 12:20 .

$ singularity exec -B $PWD ncbr_opencravat_latest.sif oc util mergesqlite cravat_chr21.fixed.sqlite cravat_chr22.fixed.sqlite -o merged.sqlite Copying cravat_chr21.fixed.sqlite to merged.sqlite... Merging cravat_chr22.fixed.sqlite...

$ echo $? 0

Everything works as expected with this version of python.

— Reply to this email directly, view it on GitHub https://github.com/KarchinLab/open-cravat/issues/113#issuecomment-1130234562 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ADC6Q62GM7CEDZ7JHRULCRDVKULMZANCNFSM5WF6PD2Q . You are receiving this because you are subscribed to this thread. https://github.com/notifications/beacon/ADC6Q67TRRODVHUSSXUPXVLVKULMZA5CNFSM5WF6PD22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOINPAFQQ.gif Message ID: @. @.> >

skchronicles commented 2 years ago

@mryaninsilico, that sounds good. Thank you for looking into this issue. I appreciate it!

kmoad commented 2 years ago

Python 3.6 is not supported now, so updating makes sense. Looks like sqlite3.connect in python3.7 will accept path-like objects. So changing required python to that would work.

https://docs.python.org/3.7/library/sqlite3.html#sqlite3.connect

However, python3.7 loses support on 2023-06-07, https://www.python.org/downloads/

So, I think it makes sense to update to 3.8.

Done in commit https://github.com/KarchinLab/open-cravat/commit/963971ee433f8c0c2e4a44a27ae5545dfd676b08