easybuild-framework icon indicating copy to clipboard operation
easybuild-framework copied to clipboard

added function get_software_version_major_minor

Open mboisson opened this issue 4 years ago • 5 comments

mboisson avatar Apr 22 '20 21:04 mboisson

Question: Have you checked how the %(version_major_minor)s is implemented? It might already have a function like that or it can use this function.

Alternative to consider: Adding a parameter to get_software_version. I imagine an enum like full, major_minor, major.

Also: What is the intended behavior for non-standard version? like for foss-2019a? This should be documented and tested. I'd make it a runtime error as there is only full but no major, minor or anything else

Flamefire avatar Apr 23 '20 08:04 Flamefire

My main goal was to avoid duplicating this code in easyblocks :

[mboisson@build-node easybuild]$ grep -r 'pyshortver =' .
./easyblocks/b/blender.py:            pyshortver = '.'.join(get_software_version('Python').split('.')[:2])
./easyblocks/generic/pythonpackage.py:                pyshortver = '.'.join(python_version.split('.')[:2])
./easyblocks/o/openbabel.py:            pyshortver = '.'.join(get_software_version('Python').split('.')[:2])
./easyblocks/p/python.py:        self.pyshortver = '.'.join(self.version.split('.')[:2])
./easyblocks/r/root.py:                pyshortver = '.'.join(get_software_version('Python').split('.')[:2])
./easyblocks/t/tensorflow.py:            pyshortver = '.'.join(get_software_version('Python').split('.')[:2])
./easyblocks/v/vmd.py:        pyshortver = '.'.join(get_software_version('Python').split('.')[:2])

I did not consider making it more robust, or behave differently.

mboisson avatar Apr 23 '20 13:04 mboisson

Once this get merged, I can make a PR for easyblocks to replace these :

easybuild/easyblocks/b/blender.py:            pyshortver = '.'.join(get_software_version('Python').split('.')[:2])
easybuild/easyblocks/d/dolfin.py:            pyver = '.'.join(get_software_version('Python').split('.')[:2])
easybuild/easyblocks/g/gamess_us.py:            fortran_ver = '.'.join(get_software_version('GCC').split('.')[:2])
easybuild/easyblocks/n/namd.py:            tclversion = '.'.join(get_software_version('Tcl').split('.')[0:2])
easybuild/easyblocks/n/nwchem.py:            pyver = '.'.join(get_software_version('Python').split('.')[0:2])
easybuild/easyblocks/o/openbabel.py:            pyshortver = '.'.join(get_software_version('Python').split('.')[:2])
easybuild/easyblocks/q/qscintilla.py:            pyshortver = '.'.join(get_software_version('Python').split('.')[:2])
easybuild/easyblocks/r/root.py:                pyshortver = '.'.join(get_software_version('Python').split('.')[:2])
easybuild/easyblocks/r/rosetta.py:            cxx_ver = '.'.join(get_software_version('GCC').split('.')[:2])
easybuild/easyblocks/t/tensorflow.py:            pyshortver = '.'.join(get_software_version('Python').split('.')[:2])
easybuild/easyblocks/v/vmd.py:        pyshortver = '.'.join(get_software_version('Python').split('.')[:2])

mboisson avatar Apr 24 '20 14:04 mboisson

I see that there are similar patterns that do not use get_software_version :

[mboisson@build-node easybuild-easyblocks]$ grep -r "'\.'.join"  easybuild | grep -v get_software_version
easybuild/easyblocks/a/abaqus.py:            custom_paths['dirs'].append('%s-%s' % ('.'.join(verparts[0:2]), verparts[2]))
easybuild/easyblocks/e/easybuildmeta.py:                    major_minor_version = '.'.join(self.version.split('.')[:2])
easybuild/easyblocks/g/geant4.py:                name = 'geant4-%s' % '.'.join(version_parts[:2] + [version_parts[-1][-1]])
easybuild/easyblocks/g/geant4.py:        g4version = '.'.join(self.version.split('.')[:2])
easybuild/easyblocks/g/gromacs.py:                major_minor_version = '.'.join(self.version.split('.')[:2])
easybuild/easyblocks/generic/pythonpackage.py:                pyshortver = '.'.join(python_version.split('.')[:2])
easybuild/easyblocks/generic/intelbase.py:            majver = '.'.join(self.version.split('.')[:-1])
easybuild/easyblocks/i/imkl.py:                            fn = '.'.join(ff[:-1]) + '_pic.' + ff[-1]
easybuild/easyblocks/m/mathematica.py:        shortver = '.'.join(self.version.split('.')[:2])
easybuild/easyblocks/n/namd.py:            self.charm_subdir = '.'.join(os.path.basename(charm_tarballs[0]).split('.')[:-1])
easybuild/easyblocks/p/python_meep.py:        shortver = '.'.join(self.version.split('.')[0:2])
easybuild/easyblocks/p/python.py:    postfix = os.path.join('lib', 'python'+'.'.join(map(str,sys.version_info[:2])), 'site-packages')
easybuild/easyblocks/p/python.py:        self.pyshortver = '.'.join(self.version.split('.')[:2])
easybuild/easyblocks/p/python.py:            tcltk_maj_min_ver = '.'.join(tclver.split('.')[:2])
easybuild/easyblocks/p/python.py:            if tcltk_maj_min_ver != '.'.join(tkver.split('.')[:2]):
easybuild/easyblocks/r/ruby.py:        majver = '.'.join(self.version.split('.')[:2])
easybuild/easyblocks/r/rosetta.py:            cxx_ver = '.'.join(get_icc_version().split('.')[:2])
easybuild/easyblocks/t/tensorflow.py:            cuda_maj_min_ver = '.'.join(cuda_version.split('.')[:2])
easybuild/easyblocks/t/tensorflow.py:                    'TF_CUBLAS_VERSION': '.'.join(cublas_ver_parts),
easybuild/easyblocks/t/tensorflow.py:                cudnn_maj_min_patch_ver = '.'.join(cudnn_version.split('.')[:3])
easybuild/easyblocks/w/wxpython.py:        majver = '.'.join(self.version.split('.')[:2])
easybuild/easyblocks/w/wxpython.py:            majver = '.'.join(self.version.split('.')[:2])

Maybe this is worth a slightly more generic function that would take a version number, rather than automatically use get_software_version ?

mboisson avatar Apr 24 '20 14:04 mboisson

@mboisson Yes, making it a bit more generic makes sense to me.

It could use get_software_version if name is specified, or use the specified version instead (or return None or raise an error if neither is provided).

Also add a test that cover the case that @Flamefire mentioned, where these is no minor version: either return None, or just return only the major version, whatever makes most sense to you.

boegel avatar May 19 '20 12:05 boegel