Updating DNA2Vec to work with Python 3.7, and to work on Windows 10. #15

LightenedLimited · 2020-05-13T12:18:26Z

A slight issue that I haven't gotten around to yet is fixing the memory usage, as the packages "resource" is a Unix specific package.

kclejeune · 2020-05-21T16:56:16Z

A slight issue that I haven't gotten around to yet is fixing the memory usage, as the packages "resource" is a Unix specific package.

Could you elaborate on the memory issue? I'm looking into using this in a unix environment and might be able to help.

LightenedLimited · 2020-05-21T17:08:01Z

Well, the original author uses the “resource” package to be able to monitor the RAM and CPU usage, but it is an UNIX specific library. If you look at attic_util/util.py, I commented out resources (all it did was return the memory usage) and just returned 0 instead on my branch. If you want to use this feature, just uncomment the relevant areas. By the way, tell me how it goes, I’ve only tested it on windows.

…

On Thursday, May 21, 2020, Kennan LeJeune ***@***.***> wrote: A slight issue that I haven't gotten around to yet is fixing the memory usage, as the packages "resource" is a Unix specific package. Could you elaborate on the memory issue? I'm looking into using this in a unix environment and might be able to help. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#15 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGKLDQU2OG6OXFIMVFSP7XLRSVML5ANCNFSM4M7WJTBA> .

pnpnpn

Can you be more specific about what's incompatible in python 3.7?

pnpnpn · 2020-05-21T17:11:35Z

attic_util/util.py

@@ -1,6 +1,6 @@
 import random
 import string
-import resource
+# import resource


Instead of commenting this out, you can use the try and catch ImportError pattern: https://stackoverflow.com/questions/3131217/error-handling-when-importing-modules

You can move this within memory_usage

Sure, I'll work on it when I get the chance.

pnpnpn · 2020-05-21T17:15:44Z

requirements.txt

-tox==2.7.0
-tox-pyenv==1.0.3
-virtualenv==15.1.0
+arrow


please pin the versions. This will install the latest versions, which may break the code.

This was one of the issues when moving up to Python 3.7. Several of the older versions of the libraries were incompatible with Python 3.7, so installing the latest versions would definitely reduce incompatibilities.

I understand that these older versions are incompatible with python 3.7. A couple of months from now, when there are upgrades to these libraries, the API for the new versions may not be backward-compatible to the current code.

Pinning the versions to new version numbers ensure that this is at least compatible to the version you tested on, which is python 3.7. You can see the versions of your current environment with pip freeze

kclejeune · 2020-05-21T17:20:31Z

Can you be more specific about what's incompatible in python 3.7?

The existing version of requirements.txt does not correctly install dependencies and causes an error when building with wheels. I think I've mostly resolved this in my pull request.

kclejeune · 2020-05-21T17:26:15Z

dna2vec/multi_k_model.py

 class SingleKModel:
    def __init__(self, model):
        self.model = model
        self.vocab_lst = sorted(model.vocab.keys())

 class MultiKModel:
    def __init__(self, filepath):
-        self.aggregate = word2vec.Word2Vec.load_word2vec_format(filepath, binary=False)
+        self.aggregate = gensim.models.KeyedVectors.load_word2vec_format(filepath, binary=False)


@pnpnpn I'm not sure if this is actually broken in python 3.7, but Word2Vec has definitely been deprecated so this is probably still a good idea.

pnpnpn · 2020-05-29T15:53:59Z

scripts/train_dna2vec.py

@@ -52,7 +52,7 @@ def train(self, kmer_seq_generator):

    def write_vec(self):
        out_filename = '{}.w2v'.format(self.out_fileroot)
-        self.model.wv.save_word2vec_format(out_filename, binary=False)
+        self.model.wv.save_word2vec_format("."+out_filename, binary=False)


Why do you prepend with .? This would make it a hidden file in linux.

I must've not noticed that change as I was working on Windows. I'll change it in the latest push.

pnpnpn · 2020-05-29T15:59:01Z

requirements.txt

-tox==2.7.0
-tox-pyenv==1.0.3
-virtualenv==15.1.0
+arrow


I understand that these older versions are incompatible with python 3.7. A couple of months from now, when there are upgrades to these libraries, the API for the new versions may not be backward-compatible to the current code.

Pinning the versions to new version numbers ensure that this is at least compatible to the version you tested on, which is python 3.7. You can see the versions of your current environment with pip freeze

pnpnpn · 2020-06-03T16:13:10Z

dna2vec/multi_k_model.py

 from gensim import matutils

+import gensim


The train_dna2vec.py also imports word2vec. Would the upgrade of gensim break the training of that file? Have you try training?

pnpnpn · 2020-06-03T16:13:52Z

scripts/train_dna2vec.py

@@ -132,7 +132,7 @@ def main():
            args.kmer_fragmenter))

    out_txt_filename = '{}.txt'.format(out_fileroot)
-    with open(out_txt_filename, 'w') as summary_fptr:
+    with open(out_txt_filename, 'w+') as summary_fptr:


Why the w+ change?

pnpnpn · 2020-06-03T16:14:07Z

dna2vec/multi_k_model.py

            print(header_str, file=fptr)
            for vocab in vocabs:
                vec_str = ' '.join("%f" % val for val in self.aggregate[vocab])
                print('{} {}'.format(vocab, vec_str), file=fptr)
            fptr.flush()
-            return SingleKModel(word2vec.Word2Vec.load_word2vec_format(fptr.name, binary=False))
+            open(fptr.name, "rb")


What's the purpose for this?

pnpnpn · 2020-06-03T16:15:04Z

dna2vec/multi_k_model.py

@@ -25,6 +27,7 @@ def __init__(self, filepath):
        self.data = {}
        for k in range(self.k_low, self.k_high + 1):
            self.data[k] = self.separate_out_model(k)
+        print(len(self.data))


Is this for debugging? Please remove.

pnpnpn · 2020-06-03T16:15:59Z

dna2vec/multi_k_model.py

@@ -4,17 +4,19 @@
 import tempfile
 import numpy as np

-from gensim.models import word2vec
+# from gensim.models import word2vec


You can just delete this.

LightenedLimited added 2 commits May 13, 2020 14:13

Forked to work with Windows, Python 3.7

f797972

Reorganizing for file perservation

6f4134e

pnpnpn requested changes May 21, 2020

View reviewed changes

kclejeune reviewed May 21, 2020

View reviewed changes

Added error logic to resources import

9868c7a

LightenedLimited requested a review from pnpnpn May 29, 2020 14:55

pnpnpn requested changes May 29, 2020

View reviewed changes

LightenedLimited added 2 commits May 31, 2020 10:50

Froze dependencies using pip freeze

81630ef

removed hidden file issue on linus

fc31397

LightenedLimited requested a review from pnpnpn May 31, 2020 08:55

pnpnpn requested changes Jun 3, 2020

View reviewed changes

kclejeune mentioned this pull request Jun 5, 2020

Fix requirements.txt for python3 dependency installation #16

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating DNA2Vec to work with Python 3.7, and to work on Windows 10. #15

Updating DNA2Vec to work with Python 3.7, and to work on Windows 10. #15

LightenedLimited commented May 13, 2020

kclejeune commented May 21, 2020

LightenedLimited commented May 21, 2020 via email

pnpnpn left a comment

pnpnpn May 21, 2020

LightenedLimited May 21, 2020

pnpnpn May 21, 2020

LightenedLimited May 21, 2020

pnpnpn May 29, 2020 •

edited

Loading

kclejeune commented May 21, 2020 •

edited

Loading

kclejeune May 21, 2020

pnpnpn May 29, 2020

LightenedLimited May 31, 2020

pnpnpn May 29, 2020 •

edited

Loading

pnpnpn Jun 3, 2020

pnpnpn Jun 3, 2020

pnpnpn Jun 3, 2020

pnpnpn Jun 3, 2020

pnpnpn Jun 3, 2020

Updating DNA2Vec to work with Python 3.7, and to work on Windows 10. #15

Are you sure you want to change the base?

Updating DNA2Vec to work with Python 3.7, and to work on Windows 10. #15

Conversation

LightenedLimited commented May 13, 2020

kclejeune commented May 21, 2020

LightenedLimited commented May 21, 2020 via email

pnpnpn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnpnpn May 29, 2020 • edited Loading

Choose a reason for hiding this comment

kclejeune commented May 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnpnpn May 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnpnpn May 29, 2020 •

edited

Loading

kclejeune commented May 21, 2020 •

edited

Loading

pnpnpn May 29, 2020 •

edited

Loading