-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating DNA2Vec to work with Python 3.7, and to work on Windows 10. #15
base: master
Are you sure you want to change the base?
Changes from 2 commits
f797972
6f4134e
9868c7a
81630ef
fc31397
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,17 +4,19 @@ | |
import tempfile | ||
import numpy as np | ||
|
||
from gensim.models import word2vec | ||
# from gensim.models import word2vec | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can just delete this. |
||
from gensim import matutils | ||
|
||
import gensim | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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. |
||
self.logger = logbook.Logger(self.__class__.__name__) | ||
|
||
vocab_lens = [len(vocab) for vocab in self.aggregate.vocab.keys()] | ||
|
@@ -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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this for debugging? Please remove. |
||
|
||
def model(self, k_len): | ||
""" | ||
|
@@ -50,10 +53,11 @@ def separate_out_model(self, k_len): | |
self.logger.warn('Missing {}-mers: {} / {}'.format(k_len, len(vocabs), 4 ** k_len)) | ||
|
||
header_str = '{} {}'.format(len(vocabs), self.vec_dim) | ||
with tempfile.NamedTemporaryFile(mode='w') as fptr: | ||
with tempfile.NamedTemporaryFile(mode='w', delete=False) as fptr: | ||
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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the purpose for this? |
||
return SingleKModel(gensim.models.KeyedVectors.load_word2vec_format(fptr.name, binary=False)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,19 @@ | ||
arrow==0.8.0 | ||
biopython==1.68 | ||
boto==2.46.1 | ||
bz2file==0.98 | ||
ConfigArgParse==0.11.0 | ||
gensim==0.13.2 | ||
Logbook==1.0.0 | ||
numpy==1.16 | ||
pep8==1.7.0 | ||
pluggy==0.4.0 | ||
py==1.4.33 | ||
pytest==3.0.7 | ||
python-dateutil==2.6.0 | ||
requests==2.20.0 | ||
scipy==0.19.0 | ||
six==1.10.0 | ||
smart-open==1.5.1 | ||
tox==2.7.0 | ||
tox-pyenv==1.0.3 | ||
virtualenv==15.1.0 | ||
arrow | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please pin the versions. This will install the latest versions, which may break the code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
biopython | ||
biopython | ||
bz2file | ||
ConfigArgParse | ||
gensim | ||
Logbook | ||
numpy | ||
pep8 | ||
pluggy | ||
py | ||
pytest | ||
python-dateutil | ||
requests | ||
six | ||
smart-open | ||
tox | ||
tox-pyenv | ||
virtualenv |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you prepend with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I must've not noticed that change as I was working on Windows. I'll change it in the latest push. |
||
|
||
def run_main(args, inputs, out_fileroot): | ||
logbook.info(' '.join(sys.argv)) | ||
|
@@ -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: | ||
with Tee(summary_fptr): | ||
logbook.StreamHandler(sys.stdout, level=log_level).push_application() | ||
redirect_logging() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of commenting this out, you can use the
try
and catchImportError
pattern: https://stackoverflow.com/questions/3131217/error-handling-when-importing-modulesYou can move this within
memory_usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll work on it when I get the chance.