-
Notifications
You must be signed in to change notification settings - Fork 295
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
Added mechanisms for retrieving tags by position within a sequence #749
Conversation
(a review would be welcome, but not yet ready for merge.) |
|
Ready for review @luizirber @camillescott @mr-c |
return NULL; | ||
} | ||
|
||
if (strlen(kmer_s) < counting->ksize()) { // @@ |
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.
What is this comment?
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.
Removed.
Py_BEGIN_ALLOW_THREADS | ||
|
||
HashIntoType kmer, kmer_f, kmer_r; | ||
kmer = _hash(kmer_s, counting->ksize(), kmer_f, kmer_r); |
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.
GCC complained about kmer being defined but not used in this function, do you see it being used in the future? If not maybe just ignore the return value and remove it from the previous line?
Thanks @luizirber - ready for re-review. |
LGTM |
Added mechanisms for retrieving tags by position within a sequence
} catch (_khmer_signal &e) { | ||
PyErr_SetString(PyExc_ValueError, e.get_message().c_str()); | ||
return NULL; | ||
} |
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.
1356-1357 lack testing coverage and won't be called ever anyhow
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.
fixed!
On Tue, Feb 10, 2015 at 11:16:55AM -0800, Michael R. Crusoe wrote:
- try {
unsigned int pos = 1;
KMerIterator kmers(seq, counting->ksize());
while (!kmers.done()) {
HashIntoType kmer = kmers.next();
if (set_contains(counting->all_tags, kmer)) {
posns.push_back(pos);
tags.push_back(kmer);
}
pos++;
}
- } catch (_khmer_signal &e) {
PyErr_SetString(PyExc_ValueError, e.get_message().c_str());
return NULL;
- }
1356-1357 lack testing coverage and won't be called ever anyhow
Reply to this email directly or view it on GitHub:
https://github.com/ged-lab/khmer/pull/749/files#r24441205
C. Titus Brown, ctbrown@ucdavis.edu
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.
Thanks; making new pull request
This is primitive functionality enabling certain sorts of queries into the graph from within Python. /cc @camillescott; at some point we should chat more about how this intersects with labeling.