Skip to content
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

fix case-sensitivity of python-facing k-mer functions #370

Open
camillescott opened this issue Apr 2, 2014 · 5 comments
Open

fix case-sensitivity of python-facing k-mer functions #370

camillescott opened this issue Apr 2, 2014 · 5 comments

Comments

@camillescott
Copy link
Member

Currently, several functions exposed in python-land are case sensitive in regards to hashing. For example,

  1. get
  2. forward_hash
  3. consume -- not sure

The result is that python code which deals directly with k-mers can fail silently on mixed-case sequence.

Assigning myself. Potential solution: wrap these functions in init.py and call upper(). Seems like a lot of overhead, though I would presume any code working with large numbers of individual k-mers would be moved to c++ land anyway.

@camillescott camillescott self-assigned this Apr 2, 2014
@ctb
Copy link
Member

ctb commented Apr 2, 2014

On Wed, Apr 02, 2014 at 10:16:41AM -0700, Camille Scott wrote:

Currently, several functions exposed in python-land are case sensitive in regards to hashing. For example,

  1. get
  2. forward_hash
  3. consume -- not sure

The result is that python code which deals directly with k-mers can fail silently on mixed-case sequence.

Assigning myself. Potential solution: wrap these functions in init.py and call upper(). Seems like a lot of overhead, though I would presume any code working with large numbers of individual k-mers would be moved to c++ land anyway.

I think the Python functions in _khmermodule should call uppercase, if
they don't do it in the lib/ land.

@mr-c mr-c added this to the 1.2+ milestone Sep 2, 2014
@mr-c mr-c added the Python label Sep 2, 2014
@mr-c
Copy link
Contributor

mr-c commented Sep 2, 2014

@camillescott Any progress?

@mr-c mr-c added the C++ label Oct 1, 2014
@mr-c
Copy link
Contributor

mr-c commented Sep 4, 2015

@camillescott @ctb Does this need to go into the known-issues list? My understanding is that it doesn't impact script users, only Python/C++ API users.

@ctb
Copy link
Member

ctb commented Sep 4, 2015 via email

@ctb
Copy link
Member

ctb commented Dec 21, 2016

The command line issue is dealt with in #1435. This issue can remain around for when we start making guarantees about the Python API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants