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

update VEP easyblock to make installation compatible with Bio::EnsEMBL::XS #1828

Merged
merged 3 commits into from
Oct 14, 2019

Conversation

lexming
Copy link
Contributor

@lexming lexming commented Oct 14, 2019

Update for vep easyblock to add sanity checks and set specific environment variables for the extension Bio::EnsEMBL::XS if it is present in the list of extension list.


if 'Bio::EnsEMBL::XS' in [ext[0] for ext in self.cfg['exts_list']]:
custom_paths['files'] += ['lib/perl%s/site_perl/%s/x86_64-linux-thread-multi/Bio/EnsEMBL/XS.pm'\
% (get_major_perl_version(), get_software_version('Perl'))]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

continuation line under-indented for visual indent

custom_paths = {
'files': ['vep'],
'dirs': ['modules/Bio/EnsEMBL/VEP'],
}

if 'Bio::EnsEMBL::XS' in [ext[0] for ext in self.cfg['exts_list']]:
custom_paths['files'] += ['lib/perl%s/site_perl/%s/x86_64-linux-thread-multi/Bio/EnsEMBL/XS.pm'\

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the backslash is redundant between brackets

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lexming Please use extend, use os.path.join and avoid breaking the long line by using a local variable:

if 'Bio::EnsEMBL::XS' in [ext[0] for ext in self.cfg['exts_list']]:
    perl_maj_ver = get_major_perl_version()
    perl_ver = get_software_version('Perl')
    perl_lib_subdir = os.path.join('lib', 'perl' + perl_maj_ver, 'site_perl', perl_ver)
    bio_ensembl_xs_perl_mod = os.path.join(perl_lib_subdir, 'x86_64-linux-thread-multi', Bio', 'EnsEMBL', 'XS.pm')
    custom_paths['files'].extend(bio_ensembl_xs_perl_mod)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with your suggestions. Thanks for the feedback.

custom_paths = {
'files': ['vep'],
'dirs': ['modules/Bio/EnsEMBL/VEP'],
}

if 'Bio::EnsEMBL::XS' in [ext[0] for ext in self.cfg['exts_list']]:
custom_paths['files'] += ['lib/perl%s/site_perl/%s/x86_64-linux-thread-multi/Bio/EnsEMBL/XS.pm'\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lexming Please use extend, use os.path.join and avoid breaking the long line by using a local variable:

if 'Bio::EnsEMBL::XS' in [ext[0] for ext in self.cfg['exts_list']]:
    perl_maj_ver = get_major_perl_version()
    perl_ver = get_software_version('Perl')
    perl_lib_subdir = os.path.join('lib', 'perl' + perl_maj_ver, 'site_perl', perl_ver)
    bio_ensembl_xs_perl_mod = os.path.join(perl_lib_subdir, 'x86_64-linux-thread-multi', Bio', 'EnsEMBL', 'XS.pm')
    custom_paths['files'].extend(bio_ensembl_xs_perl_mod)

@@ -105,9 +112,13 @@ def make_module_req_guess(self):
"""Custom guesses for environment variables (PATH, ...) for VEP."""
perl_majver = get_major_perl_version()

perl_libpath = [self.api_mods_subdir]
if 'Bio::EnsEMBL::XS' in [ext[0] for ext in self.cfg['exts_list']]:
perl_libpath += ['lib/perl%s/site_perl/%s' % (perl_majver, get_software_version('Perl'))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, please use .extend and os.path.join

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perl_maj_ver = get_major_perl_version()
perl_ver = get_software_version('Perl')
perl_libpath.extend(os.path.join('lib', 'perl' + perl_maj_ver, 'site_perl', perl_ver)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed as well 👍

@boegel boegel changed the title [vep] make compatible with Bio::EnsEMBL::XS update VEP easyblock to make installation compatible with Bio::EnsEMBL::XS Oct 14, 2019
@boegel boegel added this to the 4.x milestone Oct 14, 2019
@boegel
Copy link
Member

boegel commented Oct 14, 2019

tested with existing VEP easyconfigs and easybuilders/easybuild-easyconfigs#8733, good to go

Thanks @lexming!

@boegel boegel merged commit 2398e1e into easybuilders:develop Oct 14, 2019
@boegel boegel modified the milestones: 4.x, 4.0.1 Oct 14, 2019
@lexming lexming deleted the vep branch October 15, 2019 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants