j-andrews7 / VAMPIRE

Variant and Epigenetic anNotation for Underlying Significance and Regulation
MIT License
3 stars 0 forks source link

'join' used as method name in motif.py #40

Closed j-andrews7 closed 7 years ago

j-andrews7 commented 7 years ago

join is a default method for iterables in python, it's almost certainly not a good idea to have a method named similarly. I can't find a case where it's actually used, but it should be removed or renamed as appropriate.

Offending method starts on line 79 of motif.py.

Crumbs350 commented 7 years ago

Join was included as a good idea for future ability to process data.

join was defined as a class object method for MotifArray() class, not as a bare function. Don't believe it will cause an issue because join() is not a base python function. join() on iterators is defined for each of those individual class object types and called as object.join() just like MotifArrayObject.join() would be called. I tested by importing motif class code and verifying normal iterator joins on strings still work:

import motif
a = "Hello, "
b = "world"
"-".join([a,b])

Prints: 'Hello, -world' as expected.

Still concerned?

j-andrews7 commented 7 years ago

I guess not concerned if it doesn't cause a conflict, but feels like it may cause some unnecessary confusion if a dev wanted to use the motifs/motif module in their own code. Probably fine.

On Sat, Jun 3, 2017 at 7:33 PM, Crumbs350 notifications@github.com wrote:

Join was included as a good idea for future ability to process data.

join was defined as a class object method for MotifArray() class, not as a bare function. Don't believe it will cause an issue because join() is not a base python function https://docs.python.org/3.6/library/functions.html. join() on iterators is defined for each of those individual class object types and called as object.join() just like MotifArrayObject.join() would be called. I tested by importing motif class code and verifying normal iterator joins on strings still work:

import motif a = "Hello, " b = "world" "-".join([a,b])

Prints: 'Hello, -world' as expected.

Still concerned?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/j-andrews7/VAMPIRE/issues/40#issuecomment-306009818, or mute the thread https://github.com/notifications/unsubscribe-auth/AJwINCkccBsyMADy8i4uaiB0XMmKA2Uoks5sAftJgaJpZM4NXVI0 .