Skip to content

add support for 'eb -a rst', list available easyconfig parameters in RST format#1131

Merged
boegel merged 7 commits intoeasybuilders:developfrom
boegel:eb_a_rst
Jan 20, 2015
Merged

add support for 'eb -a rst', list available easyconfig parameters in RST format#1131
boegel merged 7 commits intoeasybuilders:developfrom
boegel:eb_a_rst

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Jan 9, 2015

This is required to easily update http://easybuild.readthedocs.org/en/latest/eb_a.html#easyconfigs-parameters and use a pretty table format (see for example https://gist.github.com/boegel/490205a6c0607bc7dfc6).

eb -a is still supported, and defaults to eb -a txt; the output has slightly changed, i.e. the descriptions are now left-aligned per group of parameters, which looks a lot better.

Once this is merged in, I'll look into supporting rst for --list-toolchains, --list-easyblocks, --help, too

@wpoely86: please review?

@hpcugentbot
Copy link
Copy Markdown

Test FAILed.

Comment thread easybuild/tools/options.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why not fix it there?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah, I should, I included this as a sort of placeholder...

The problem is that, for example, the constant CUSTOM as it is used in framework/easyconfig/default.py corresponds to the string "CUSTOM".
However, the constant CUSTOM that is imported from framework.easyconfig, as is done in easyblocks when defining custom parameters via extra_options, corresponds to (1, 'easyblock-specific'), since it's obtained from ALL_CATEGORIES in framework/easyconfig/init.py`.

That causes problems when comparing categories, as is done here, since then all categories should be of the same kind, whether they are obtained for default or extra parameters.

I can probably fix it without breaking easyblocks, but I need to think about it. :-)

@wpoely86
Copy link
Copy Markdown
Member

wpoely86 commented Jan 9, 2015

looks fine

Comment thread easybuild/tools/options.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

move to other module (and this is a funciton, not class method)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok, will move to easybuild/tools/prettyprint.py

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jan 9, 2015

Jenkins: test this please

@hpcugentbot
Copy link
Copy Markdown

Test PASSed.

@hpcugentbot
Copy link
Copy Markdown

Test PASSed.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jan 9, 2015

@stdweird, @wpoely86: remarks resolved, please rereview

@hpcugentbot
Copy link
Copy Markdown

Test PASSed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is not really acceptable. why is this not

ALL_CATEGORIES = [HIDDEN, ...]

(that would also define the order) btw.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

here's what's in easybuild/framework/easyconfig/__init__.py:

from easybuild.framework.easyconfig.default import ALL_CATEGORIES
globals().update(ALL_CATEGORIES)

ALL_CATEGORIES is only there such that the constants can be defined in the easybuild.framework.easyconfig namespace, it's not used anywhere else; the constants are directly accessible here...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but now you copy the names again, without any garantee that the keys are the as the constants.
and only because you need HIDDEN? which you could get from

from easybuild.framework.easyconfig import HIDDEN

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no, I need all of them, both in default.py itself, and potentially in easyblocks too (which is what the 'trick' in init is for)

I know no other way to make sure that the names of the constants in default.py and the ones being defined in __init__.py are the same... I'm open to suggestions.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok, i overlooked something 😄
i'm just afraid that this very error prone
maybe add a test with

names=['hidden', ...]
for ind,name in enumerate(names):
    constname=name.replace('-').upper()
    # this validates the import
    # i thinkl import returns too
    constvalue=import('easybuild.framework.easyconfig', constname)
    assert(constane, (ind-1, name))
    assert(constvalue, ALL_CATEGORIES[constname])    

(or however this works again in python 😄)

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jan 16, 2015

@stdweird: figured out a 'cleaner' solution for defining the category constants (see bf17467), in the sense that things go out of sync accidentally; I prefer this approach over a dedicated unit test

please rerererereview?

@hpcugentbot
Copy link
Copy Markdown

Test PASSed.

@stdweird
Copy link
Copy Markdown
Contributor

yes indeed, even better solution
@boegel can be merged

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jan 20, 2015

Thanks for the review @wpoely86 and @stdweird!

boegel added a commit that referenced this pull request Jan 20, 2015
add support for 'eb -a rst', list available easyconfig parameters in RST format
@boegel boegel merged commit 2565bff into easybuilders:develop Jan 20, 2015
@boegel boegel deleted the eb_a_rst branch January 20, 2015 11:04
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Jul 3, 2015

@Caylo: can you take a look at extending this to --list-toolchains (to (re)generate http://easybuild.readthedocs.org/en/latest/eb_list_toolchains.html), --list-easyblocks (to generate http://easybuild.readthedocs.org/en/latest/eb_list_easyblocks.html also in table(?) format), and maybe --help too?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants