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
Introduce function ensure_iterable_license_specs #2157
Introduce function ensure_iterable_license_specs #2157
Conversation
elif isinstance(specs, basestring): | ||
license_specs = [specs] | ||
elif isinstance(specs, (list, tuple)) and all(isinstance(x, basestring) for x in specs): | ||
license_specs = specs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would ensure a consistent return type here, either a list of a tuple...
To ensure a list, use list(specs)
here.
To ensure a tuple, use tuple(specs)
here and (None,)
and (specs,)
above.
@geimer You forgot the commit for the unit test for the new function. ;-) |
@boegel I only have two hands and a single keyboard, so my multi-threading capabilities are limited ;-) |
test/framework/type_checking.py
Outdated
self.assertEqual(ensure_iterable_license_specs('foo'), ['foo']) | ||
self.assertEqual(ensure_iterable_license_specs(['foo']), ['foo']) | ||
self.assertEqual(ensure_iterable_license_specs(['foo', 'bar']), ['foo', 'bar']) | ||
self.assertEqual(ensure_iterable_license_specs(('foo')), ['foo']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you want to test with a single-element tuple, use ('foo',)
rather than ('foo')
(which is identical to 'foo'
test/framework/type_checking.py
Outdated
self.assertErrorRegex(EasyBuildError, error_msg, ensure_iterable_license_specs, [42, 'foo']) | ||
self.assertErrorRegex(EasyBuildError, error_msg, ensure_iterable_license_specs, [['foo']]) | ||
self.assertErrorRegex(EasyBuildError, error_msg, ensure_iterable_license_specs, [(42, 'foo')]) | ||
self.assertErrorRegex(EasyBuildError, error_msg, ensure_iterable_license_specs, (42)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(42,)
@boegel Travis doesn't seem to pick up the last commit. What was the trick again? |
(close/open to wake up Travis) |
@geimer This is good to go, but I'll hold off merging it into |
Going in, thanks @geimer! |
...as a result of refactoring easybuilders/easybuild-easyblocks#1129