Additional easyconfig parameters for documentation (REVIEW) #2113

geimer · 2017-02-10T10:11:12Z

This PR implements support for additional easyconfig parameters for customizing the documentation as discussed here: https://www.mail-archive.com/[email protected]/msg03050.html
It also includes the list of installed extensions of a package in the module help output.

usage, support, contact

boegel · 2017-02-10T21:46:22Z

easybuild/framework/easyconfig/default.py

+    'docpaths': [None, "List of paths of installed documentation relative to package root dir", MODULES],
+    'docurls': [None, "List of URLs to package documentation", MODULES],
+    'support': [None, "Package-specific support/bug report address", MODULES],
+    'contact': [None, "Site contact for the package", MODULES],


hmm, this is getting a bit messy, is it not?

we should try and sort these alphabetically, although maybe also keep related ones together in groups?

for contact, don't make it specific to site contact, could also be upstream contact for this software package (e.g. a mailing list)?

Well, the idea was to separate upstream contacts (support) from site-local contacts (contact). The documentation of those two parameters already tries to make that clear. But maybe the naming is sub-optimal. I'm open for suggestions.

Hmm... I just noticed that there is already a docurls parameter defined in the MANDATORY section, though it is not (yet) enforced. I guess it hasn't been used so far?

Regarding the "messiness": Would it make sense to add a DOCUMENTATION group and add all those variables to it? Though description would then need to be in MANDATORY and DOCUMENTATION...

I think the support documentation needs to indicate that this is for upstream, not site-local support. Otherwise it's rather ambiguous.

What about upstream_contact and site_contact? This should make it clear. And since the list values are free-form text fields, one could do something like

upstream_contact = [ 'Support e-mail: [email protected]', 'Mailing list: [email protected]', 'Bugtracker: https://www.project.org/bugzilla' ] site_contact = ['Jane Admin']

Which would be rendered as

More information ================ - Homepage: https://www.project.org - Upstream contact: - Support e-mail: [email protected] - Mailing list: [email protected] - Bugtracker: https://www.project.org/bugzilla - Site contact: Jane Admin

Yes, exactly what I had in mind.

boegel · 2017-02-10T21:47:28Z

easybuild/tools/module_generator.py

+from easybuild.tools.filetools import convert_name, mkdir, read_file
+from easybuild.tools.modules import ROOT_ENV_VAR_NAME_PREFIX, modules_tool
 from easybuild.tools.utilities import quote_str
+from textwrap import wrap


this is a non-easybuild import, it should go up with the others?

boegel · 2017-02-10T21:58:15Z

easybuild/tools/module_generator.py

+            'Description',
+            '===========',
+            "%s" % description.strip(),
+        ]


this is a common pattern below, so maybe define a helper function inside of this method to ensure consistency of the format?

def _generate_help_text(self): ... def generate_section(sec_name, sec_txt): """ Generate section with given name and contents """" if sec_txt: return [ '', sec_name, '=' * len(sec_name), sec_txt.strip(), ] else: return [] # General package description (mandatory) lines = generate_section('Description', self.app.cfg['description']) # Package usage instructions (optional) lines.extend('Usage', self.app.cfg['usage']) # Additional information: homepage + (if available) doc paths/urls, support info, contacts, ... lines.extend('More information', " - Homepage: %s" % self.app.cfg['homepage']) ... extensions = ', '.join(sorted(['%s-%s' % (ext[0], ext[1]) for ext in exts_list], key=str.lower)) lines.extend('Extensions', '\n'.join(wrap(extensions, 78)))

boegel · 2017-02-10T21:59:01Z

easybuild/tools/module_generator.py

+                '',
+                "Included extensions",
+                '===================',
+                "%s" % '\n'.join(wrap(extensions, 78)),


hmm, the 78 here looks very arbitrary/harcoded... why exactly 78?

To make it fit nicely into a 80-character-wide terminal window 😁

When making this a configuration parameter, I'd also like to see all other sections to be wrapped at the given width. But this is (at least from my "I don't know much Python" perspective) by no means trivial to achieve with textwrap.wrap(). Maybe someone knows a good rst-to-ascii-text renderer that we could leverage?

boegel · 2017-02-10T21:59:34Z

easybuild/tools/module_generator.py

            "proc ModulesHelp { } {",
-            "    puts stderr { %(description)s",
+            "    puts stderr {"
+            "%s" % self._generate_help_text(),


"%s" % x is the same thing as just x

boegel · 2017-02-10T21:59:51Z

easybuild/tools/module_generator.py

        lines = [
-            "help([[%(description)s]])",
+            'help([[',
+            "%s" % self._generate_help_text(),


here too, no need for the "%s" %

boegel · 2017-02-10T22:02:08Z

easybuild/tools/module_generator.py

+        contact = self.app.cfg['contact']
+        if contact:
+            if isinstance(contact, list):
+                lines.extend([" - Site contacts:"])


drop the Site ?

pforai · 2017-02-11T18:57:39Z

I like the idea 👍

geimer · 2017-03-04T17:21:17Z

@boegel There is no test yet for the whatis/help text in the presence of extensions, and I'm not quite sure where to put it. Any suggestion? Otherwise I think this is now ready for prime time.

boegel · 2017-03-30T07:18:31Z

@geimer Can you take care of the conflict before I revisit this?

W.r.t. a test for whatis/help with extensions involved, we have toy tests that involve extensions, maybe there? Let me know if you're up for looking into that.

…o additional_doc_params

geimer · 2017-03-30T08:11:45Z

@boegel Who f?$%ed up the module_generator ;-) Conflict resolved, Travis is running.

The test checking the modulefile contents is currently using test_toy_tweaked, but could use test_toy_advanced instead. But I'm not yet convinced that this doesn't create other problems...

boegel · 2017-03-30T08:21:57Z

@geimer @damianam did an effort to sort things alphabetically in module_generator, cfr. #2132

boegel · 2017-03-30T08:23:41Z

@geimer why would using test_toy_advanced be better (or why would it create problems)?

damianam · 2017-03-30T08:47:35Z

Who f?$%ed up the module_generator

Ups ;-P. Sorting it wasn't really necessary, but working with it with methods put at random places in the file was kind of annoying, that's why I shuffled everything around.

geimer · 2017-03-30T09:30:14Z

@boegel If I'm not mistaken, test_toy_advanced is the one using extensions. But it would have to be adjusted to include the additional ec parameters which may have undesired side effects.

boegel · 2017-03-31T07:00:17Z

easybuild/framework/easyconfig/default.py


+    # MODULES documentation easyconfig parameters
+    #    (docurls is part of MANDATORY)
+    'docpaths': [None, "List of paths of installed documentation relative to package root dir", MODULES],


List of paths for documentation relative to installation directory

boegel · 2017-03-31T07:01:23Z

easybuild/framework/easyconfig/default.py

+    # MODULES documentation easyconfig parameters
+    #    (docurls is part of MANDATORY)
+    'docpaths': [None, "List of paths of installed documentation relative to package root dir", MODULES],
+    'examples': [None, "Free-form text with examples", MODULES],


... with examples on using the software

boegel · 2017-03-31T07:01:36Z

easybuild/framework/easyconfig/default.py

+    #    (docurls is part of MANDATORY)
+    'docpaths': [None, "List of paths of installed documentation relative to package root dir", MODULES],
+    'examples': [None, "Free-form text with examples", MODULES],
+    'site_contact': [None, "String/list of strings with site contacts for the package", MODULES],


plural: site_contacts

also: s/package/software/

boegel · 2017-03-31T07:02:03Z

easybuild/framework/easyconfig/default.py

+    'site_contact': [None, "String/list of strings with site contacts for the package", MODULES],
+    'upstream_contact': [None, ("String/list of strings with upstream contact addresses "
+                                "(e.g., support e-mail, mailing list, bugtracker)"), MODULES],
+    'usage': [None, "Usage instructions for the package", MODULES],


s/package/software/

boegel · 2017-03-31T07:02:31Z

easybuild/framework/easyconfig/default.py

+    'upstream_contact': [None, ("String/list of strings with upstream contact addresses "
+                                "(e.g., support e-mail, mailing list, bugtracker)"), MODULES],
+    'usage': [None, "Usage instructions for the package", MODULES],
+    'whatis': [None, "List of brief (one line) package description entries", MODULES],


s/package/software/

boegel · 2017-03-31T07:23:21Z

easybuild/tools/module_generator.py

+
+        return whatis
+
+    def _generate_extension_list(self):


sort these methods alphabetically, let's not let @damianam's efforts go to waste ;)

boegel · 2017-03-31T07:24:06Z

easybuild/tools/module_generator.py

+        Generate a string with a comma-separated list of extensions.
+        """
+        exts_list = self.app.cfg['exts_list']
+        extensions = ', '.join(sorted(['%s-%s' % (ext[0], ext[1]) for ext in exts_list], key=str.lower))


s/(ext[0], ext[1])/ext[:2]/

TypeError: not enough arguments for format string

Now using '-'.join(ext[:2]) for ext in ... but I'm not sure it helps w.r.t. readablity...

boegel · 2017-03-31T07:24:47Z

easybuild/tools/module_generator.py

            # - 'conflict Compiler/GCC/4.8.2/OpenMPI' for 'Compiler/GCC/4.8.2/OpenMPI/1.6.4'
            lines.extend(['', "conflict %s" % os.path.dirname(self.app.short_mod_name)])

+        whatis = self._generate_whatis_lines()


just move this inline, only used in one place?

boegel · 2017-03-31T07:25:33Z

easybuild/tools/module_generator.py

            # conflict on 'name' part of module name (excluding version part at the end)
            lines.extend(['', 'conflict("%s")' % os.path.dirname(self.app.short_mod_name)])

+        whatis = self._generate_whatis_lines()


same here, move this inline?

boegel · 2017-03-31T07:26:19Z

test/framework/module_generator.py


-        gzip_txt = "gzip (GNU zip) is a popular data compression program as a replacement for compress "
-        gzip_txt += "- Homepage: http://www.gzip.org/"
+        gzip_txt = "gzip (GNU zip) is a popular data compression program as a replacement for compress"


rename gzip_txt to descr?

boegel · 2017-05-02T11:13:56Z

lgtm, going in, thanks for your hard work and patience on this @geimer!

geimer added 8 commits February 10, 2017 10:24

Add support for additional easyconfig parameters for documentation:

b4098a3

usage, support, contact

Include list of extensions in help output

836b61e

Style fixes

e5d642a

Add support for docpaths and docurls easyconfig parameters

b7cf1f3

Tweaking

1f9a152

Adjust existing unit tests

7a74752

Case-insensistive sorting of extensions

d44debf

Use all documentation ec parameters for tweaked toy build in unit tests

2e51c8c

boegel requested changes Feb 10, 2017

View reviewed changes

boegel added this to the 3.2.0 milestone Feb 10, 2017

geimer added 10 commits February 18, 2017 13:36

Parameter 'docurls' was already there (in section MANDATORY)

2fa6786

Replace 'support' parameter by 'upstream_contact'

503f4ed

Replace parameter 'contact' by 'site_contact'

9923d99

Addressed comments

13245c8

Fix test failures

7174fa9

Fix expected test output

68c7897

Add 'examples' easyconfig parameter

f2f0f3c

Tweaked parameter descriptions

e3e3852

Split description and homepage in 'whatis' lines

4a589db

Add whatis line with list of extensions

502fdb9

geimer changed the title ~~Additional easyconfig parameters for documentation (WIP)~~ Additional easyconfig parameters for documentation (REVIEW) Mar 4, 2017

Merge branch 'develop' of github.com:hpcugent/easybuild-framework int…

c5db8a6

…o additional_doc_params

boegel requested changes Mar 31, 2017

View reviewed changes

Addressed comments

59bfa95

boegel approved these changes May 2, 2017

View reviewed changes

boegel merged commit 7f0bf95 into easybuilders:develop May 2, 2017

boegel mentioned this pull request May 6, 2017

fix crash during module generation when '%' character is used in description #2209

Merged

geimer deleted the additional_doc_params branch October 30, 2017 10:32

Additional easyconfig parameters for documentation (REVIEW) #2113

Additional easyconfig parameters for documentation (REVIEW) #2113

Uh oh!

Conversation

geimer commented Feb 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pforai commented Feb 11, 2017

Uh oh!

geimer commented Mar 4, 2017

Uh oh!

boegel commented Mar 30, 2017

Uh oh!

geimer commented Mar 30, 2017

Uh oh!

boegel commented Mar 30, 2017

Uh oh!

boegel commented Mar 30, 2017

Uh oh!

damianam commented Mar 30, 2017

Uh oh!

geimer commented Mar 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boegel commented May 2, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects