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

add infernal-cm datatype #4977

Merged
merged 19 commits into from Jan 11, 2018

Conversation

Projects
None yet
5 participants
@mmiladi
Copy link
Contributor

mmiladi commented Nov 10, 2017

Co-variance Models (CM) datatype from Infernal RNA package

mmiladi added some commits Nov 10, 2017

@galaxybot galaxybot added the triage label Nov 10, 2017

@galaxybot galaxybot added this to the 18.01 milestone Nov 10, 2017

mmiladi added some commits Nov 10, 2017

@bgruening
Copy link
Member

bgruening left a comment


def sniff(self, filename):
with open('myfile.txt', 'r') as f:
first_line = f.readline()

This comment has been minimized.

Copy link
@bgruening

bgruening Nov 10, 2017

Member

you could use here read(11) to read the first characters and compare it with your string

This comment has been minimized.

Copy link
@mmiladi

mmiladi Nov 10, 2017

Author Contributor

'myfile.txt' must be wrong or filename is unnecessary. agree?

This comment has been minimized.

Copy link
@bgruening

bgruening Nov 10, 2017

Member

Yes this needs to be filename I guess.

with open('myfile.txt', 'r') as f:
first_line = f.readline()

if ("INFERNAL1/a" in first_line):

This comment has been minimized.

Copy link
@bgruening

bgruening Nov 10, 2017

Member

no brackets needed

class Infernal_CM_1_1(Text):
file_ext = "cm"

MetadataElement(name="number_of_models", default=0, desc="Number of covariance models", readonly=True, visible=True, optional=True, no_value=0)

This comment has been minimized.

Copy link
@bgruening

bgruening Nov 10, 2017

Member

We don't calculate the number of models since the removal of grep, so we can remove it I guess and the import as well.

This comment has been minimized.

Copy link
@mmiladi

mmiladi Nov 10, 2017

Author Contributor

set_peek also works with dataset.metadata. could that be the reason for initing MetadataElement object?

def set_peek(self, dataset, is_multi_byte=False):
if not dataset.dataset.purged:
dataset.peek = get_file_peek(dataset.file_name, is_multi_byte=is_multi_byte)
if (dataset.metadata.number_of_models == 1):

This comment has been minimized.

Copy link
@bgruening

bgruening Nov 10, 2017

Member

remove brackets

mmiladi added some commits Nov 10, 2017

def set_peek(self, dataset, is_multi_byte=False):
if not dataset.dataset.purged:
dataset.peek = get_file_peek(dataset.file_name, is_multi_byte=is_multi_byte)
if dataset.metadata.number_of_models == 1:

This comment has been minimized.

Copy link
@mmiladi

mmiladi Nov 10, 2017

Author Contributor

Wouldn't line17 be needed for setting the metadata here? Although idk if set_peek is overall needed or not

This comment has been minimized.

Copy link
@bgruening

bgruening Nov 17, 2017

Member

Yes you can remove the model count completely, if we do not count anymore.

This comment has been minimized.

Copy link
@mmiladi

mmiladi Nov 17, 2017

Author Contributor

I guess, we count number of models in line 204. It's a useful info.

@@ -11,6 +11,87 @@
log = logging.getLogger(__name__)


class Infernal_CM_1_1(Text):

This comment has been minimized.

Copy link
@bgruening

bgruening Nov 17, 2017

Member

Let's call this class InfernalCM and add a MetadataElement(name="version", ...) where we store the Infernal CM version. This way we just need one class for future versions of this file format.

This comment has been minimized.

Copy link
@mmiladi

mmiladi Nov 17, 2017

Author Contributor

Looks fine? Should it be optional?
MetadataElement(name="cm_version", default=1.1, desc="CM Infernal version ", readonly=True, visible=True, optional=True, no_value=0)

This comment has been minimized.

Copy link
@bgruening

bgruening Nov 17, 2017

Member

desc could be "Infernal Covariance Model version" and name simply "version"?
You can later access this information via $file.metadata.version

This comment has been minimized.

Copy link
@mmiladi

mmiladi Nov 17, 2017

Author Contributor

CM versions might not be compatible. How expose the version info to the user?

@mmiladi

This comment has been minimized.

Copy link
Contributor Author

mmiladi commented Nov 17, 2017

Todos:

  • Check model count functionality works
  • Check if set_peek needs to be updated for CM version Metadata
@mvdbeek

This comment has been minimized.

Copy link
Member

mvdbeek commented Jan 3, 2018

@mmiladi I have put this in WIP and changed the milestone to 18.05 based on your todo list. If you have some more commits before the feature freeze we can of course consider it for 18.01

@mvdbeek mvdbeek modified the milestones: 18.01, 18.05 Jan 3, 2018

@mmiladi

This comment has been minimized.

Copy link
Contributor Author

mmiladi commented Jan 5, 2018

@mvdbeek sorry for the delay. Checks are now finished. If it looks good to you and @bgruening, it's ready for merge.

with open(filename, 'r') as f:
first_line = f.readline()

if first_line.startswith("INFERNAL"):

This comment has been minimized.

Copy link
@bgruening

bgruening Jan 5, 2018

Member

You could do return first_line.startswith("INFERNAL")

This comment has been minimized.

Copy link
@mmiladi

mmiladi Jan 5, 2018

Author Contributor

Done. Imo it takes the same time for a computer to run but twice the time for a human to understand ;-)


def set_meta(self, dataset, **kwd):
"""
Set the number of models in dataset.

This comment has been minimized.

Copy link
@bgruening

bgruening Jan 5, 2018

Member

and the version number...

@@ -0,0 +1,4114 @@
INFERNAL1/a [1.1 | April 2012]
NAME tRNA
ACC RF00005

This comment has been minimized.

Copy link
@bgruening

bgruening Jan 5, 2018

Member

Can we use a smaller file?

This comment has been minimized.

Copy link
@mmiladi

mmiladi Jan 5, 2018

Author Contributor

Halved now :)

mmiladi added some commits Jan 5, 2018

@bgruening

This comment has been minimized.

Copy link
Member

bgruening commented Jan 8, 2018

looks good to me. @mvdbeek?

@mvdbeek mvdbeek added status/review and removed status/WIP labels Jan 11, 2018

@mvdbeek mvdbeek modified the milestones: 18.05, 18.01 Jan 11, 2018

@mvdbeek mvdbeek merged commit c7581f8 into galaxyproject:dev Jan 11, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mvdbeek

This comment has been minimized.

Copy link
Member

mvdbeek commented Jan 11, 2018

Thanks @mmiladi !

@mmiladi

This comment has been minimized.

Copy link
Contributor Author

mmiladi commented Jan 11, 2018

Thank you @mvdbeek !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.