Cufflinks easyblock contributions #53

Merged
merged 11 commits into from Jan 17, 2013

Conversation

Projects
None yet
4 participants
Contributor

claczny commented Dec 7, 2012

enjoy,

Signed-off-by: Cedric Laczny cedric.laczny@uni.lu

Cedric Laczny contrib_Bioinfo_easyblocks_by_Cedric
Signed-off-by: Cedric Laczny <cedric.laczny@uni.lu>
a2ef3ec

@JensTimmerman JensTimmerman commented on an outdated diff Dec 7, 2012

easybuild/easyblocks/c/cufflinks.py
+import re
+import os
+import shutil
+import sys
+
+from easybuild.easyblocks.generic.configuremake import ConfigureMake
+
+class EB_Cufflinks(ConfigureMake):
+ """
+ Support for building cufflinks (Transcript assembly, differential expression, and differential regulation for RNA-Seq)
+ """
+ def patch_step(self):
+ """
+ First we need to rename a few things, s.a. http://wiki.ci.uchicago.edu/Beagle/BuildingSoftware -> "Cufflinks"
+ """
+ build_dir = os.getcwd()
@JensTimmerman

JensTimmerman Dec 7, 2012

Owner

this indent needs to be fixed, will not work this way.

Owner

JensTimmerman commented Dec 7, 2012

Something is wrong with the indentations, since this is python, it really needs to be fixed...

Cedric Laczny added some commits Dec 7, 2012

Cedric Laczny Changed tabs to spaces
Signed-off-by: Cedric Laczny <cedric.laczny@uni.lu>
ea24de6
Cedric Laczny fixed tabs to ... nothing
Signed-off-by: Cedric Laczny <cedric.laczny@uni.lu>
5e0a60f
Collaborator

fgeorgatos commented Dec 7, 2012

re-review

Owner

JensTimmerman commented Dec 7, 2012

the indentation inside the patch_step function is still wrong.

@JensTimmerman JensTimmerman and 3 others commented on an outdated diff Dec 7, 2012

easybuild/easyblocks/c/cufflinks.py
+# Copyright:: Copyright (c) 2012 University of Luxembourg / LCSB
+# Author:: Cedric Laczny <cedric.laczny@uni.lu>, Fotis Georgatos <fotis.georgatos@uni.lu>
+# License:: MIT/GPL
+# File:: $File$
+# Date:: $Date$
+
+import fileinput
+import glob
+import re
+import os
+import shutil
+import sys
+
+from easybuild.easyblocks.generic.configuremake import ConfigureMake
+
+class EB_Cufflinks(ConfigureMake):
@JensTimmerman

JensTimmerman Dec 7, 2012

Owner

You need a configure step where you check if boost is loaded, since it will not build without boost.
if not get_software_root("Boost"): fail

@claczny

claczny Dec 7, 2012

Contributor

it's a dependency already, no?
i would assume that specifying it further as builddependency is overkill, isn't it?

If this was required, it would have to apply for all deps I guess...

@JensTimmerman

JensTimmerman Dec 7, 2012

Owner

Well, I have to agree here, but @boegel has other views

@fgeorgatos

fgeorgatos Dec 7, 2012

Collaborator

Just a comment: about everything that adds to the SLOC, better it gives some concrete advantage, otherwise it will become an extra maintenance point and we'll pay a price for it later on... (thinking of the whole family of easyblocks together)

@boegel

boegel Dec 8, 2012

Owner

I try to make a habit of it the explicitly check for required dependencies, like Boost is in this this.

Very often, dependencies are optional, and just enhance the features of what's being built.

I don't see this as something that's really required, but I consider it good practice in the easyblocks I write. So, up to you.

@claczny

claczny Dec 8, 2012

Contributor

Do you have a reference of an easyblock that integrates this so I can have a look? Currently, I don't know what I should favor because your point (@boegel) sounds also valid to me.

@boegel

boegel Dec 8, 2012

Owner

An example (for Boost, coincidentally) can be seen in the easyblock for Armadillo: https://github.com/hpcugent/easybuild-easyblocks/blob/master/easybuild/easyblocks/a/armadillo.py .

Cedric Laczny fixed indentation inside patch step
Signed-off-by: Cedric Laczny <cedric.laczny@uni.lu>
b8d1346

@claczny claczny and 2 others commented on an outdated diff Dec 7, 2012

easybuild/easyblocks/c/cufflinks.py
+# Date:: $Date$
+
+import fileinput
+import glob
+import re
+import os
+import shutil
+import sys
+
+from easybuild.easyblocks.generic.configuremake import ConfigureMake
+
+class EB_Cufflinks(ConfigureMake):
+ """
+ Support for building cufflinks (Transcript assembly, differential expression, and differential regulation for RNA-Seq)
+ """
+ def patch_step(self):
@claczny

claczny Dec 7, 2012

Contributor

fixed!

@JensTimmerman

JensTimmerman Dec 7, 2012

Owner

Your docstring still isn't :-p

@claczny

claczny Dec 7, 2012

Contributor

Done :)
On 7 Dec 2012, at 17:22, Jens Timmerman wrote:

In easybuild/easyblocks/c/cufflinks.py:

+# Date:: $Date$
+
+import fileinput
+import glob
+import re
+import os
+import shutil
+import sys
+
+from easybuild.easyblocks.generic.configuremake import ConfigureMake
+
+class EB_Cufflinks(ConfigureMake):

  • """
  • Support for building cufflinks (Transcript assembly, differential expression, and differential regulation for RNA-Seq)
  • """
  • def patch_step(self):

Your docstring still isn't :-p


Reply to this email directly or view it on GitHub.

Cedric Laczny,
PhD Student

UNIVERSITÉ DU LUXEMBOURG

LUXEMBOURG CENTRE FOR SYSTEMS BIOMEDICINE
Campus Belval | House of Biomedicine
7, avenue des Hauts-Fourneaux
L-4362 Esch-sur-Alzette
T +352 46 66 44 6398
F +352 46 66 44 6949
cedric.laczny@uni.lu http://lcsb.uni.lu


This message is confidential and may contain privileged information. It is intended for the named recipient only. If you receive it in error please notify me and permanently delete the original message and any copies.

@boegel

boegel Dec 8, 2012

Owner

Add an empty line before def patch_step, to separate class docstring from the rest.

Cedric Laczny aaargh, spaces, spaces...
Signed-off-by: Cedric Laczny <cedric.laczny@uni.lu>
74fc791

@boegel boegel commented on an outdated diff Dec 8, 2012

easybuild/easyblocks/o/oases.py
+ pass
+
+ def build_step(self):
+ """
+ Needs to get the path of the build-dir of velvet -> requires headers
+ """
+ builddep = self.cfg['builddependencies']
+ # assert that it only has ONE builddep specified
+ assert len(builddep) == 1
+
+ #srcdir = self.cfg['startfrom']
+ srcdir = self.builddir
+
+ print builddep
+ velvet = builddep[0][0]
+ velvetver = builddep[0][1]
@boegel

boegel Dec 8, 2012

Owner

You shouldn't set Velvet as build dependency if you just need access to it's sources.

Just include the Velvet tarball in sources like you did in hpcugent/easybuild-easyconfigs#52, and then use something like this (untested, but should do the trick I think):

velvet_src_dir = [d for d in os.listdir(self.builddir) if d.startswith('velvet')][0]

And then adjust the below to

self.cfg.update('makeopts', 'VELVET_DIR=%s' % os.path.join(self.builddir, velvet_src_dir))
super(EB_Oases, self).build_step()

Calling the build_step of the parent is nicer, e.g. it will also enabled building in parallel and maybe other cool stuff done by the ConfigureMake easyblock, so why not use it.

@boegel

boegel Dec 8, 2012

Owner

Based on https://github.com/hpcugent/easybuild-easyconfigs/pull/52/files#r2357131, what you actually want is probably:

self.cfg.update('makeopts', 'VELVET_DIR=%s' % get_software_root('Velvet'))
super(EB_Oases, self).build_step()

And then simply specify Velvet as build dependency. You should verify that Oases really doesn't require Velvet as runtime though (e.g. Velvet libraries).

Cedric Laczny and others added some commits Dec 10, 2012

Cedric Laczny Integrated suggestions about removing the patch, dependency testing i…
…n the easyblock and formatting

Signed-off-by: Cedric Laczny <cedric.laczny@uni.lu>
323bdc5
Cedric Laczny removed oases due to complications
Signed-off-by: Cedric Laczny <cedric.laczny@uni.lu>
e300184
@boegel boegel Merge pull request #60 from boegel/ALADIN
support for building and installing ALADIN
cc70a8d
@boegel boegel Merge branch 'contrib_Bioinfo_easyblocks_by_Cedric' of github.com:cla…
…czny/easybuild-easyblocks into contrib_Bioinfo_easyblocks_by_Cedric
ad50ad7
@boegel boegel fix indentation (don't use tabs), style fixes 79654ab

boegel referenced this pull request in claczny/easybuild-easyblocks Jan 17, 2013

Merged

fix indentation issues in Cufflinks easyblock #1

Cedric Laczny Merge pull request #1 from boegel/contrib_Bioinfo_easyblocks_by_Cedric
fix indentation issues in Cufflinks easyblock
8bc4dcd

@boegel boegel added a commit that referenced this pull request Jan 17, 2013

@boegel boegel Merge pull request #53 from claczny/contrib_Bioinfo_easyblocks_by_Cedric
Cufflinks easyblock contributions
f974e8d

@boegel boegel merged commit f974e8d into hpcugent:develop Jan 17, 2013

boegel referenced this pull request Jan 18, 2013

Closed

added oases.py easyblock #79

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