Skip to content

Dev/improve configurable documentation #406

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.

Sign up for GitHub

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

Open
as6520 wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Dev/improve configurable documentation #406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cbdf861
Add an example for overriding get_default_config
Mar 6, 2020
b7c8723
Update release notes
Mar 6, 2020
cc08242
Add suggested changes
Mar 6, 2020
8827dc2
Fix python 2.7 unit test failure
May 3, 2020
5272792
Merge branch 'master' into dev/improve-configurable-documentation
May 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/release_notes/pending_release.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@ Fixes
Documentation

* Add missing reference to v0.13.0 change notes.

* Add an example for overriding get_default_config
21 changes: 19 additions & 2 deletions python/smqtk/utils/configuration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,27 @@ def get_default_config(cls):
turning those argument names into configuration dictionary keys. If any
of those arguments have defaults, we will add those values into the
configuration dictionary appropriately. The dictionary returned should
only contain JSON compliant value types.
only contain JSON compliant value types. If the default arguments are
not JSON compliant then the function should be overriden to convert
the convert the default to a JSON compliant stand in.

It is not be guaranteed that the configuration dictionary returned
from this method is valid for construction of an instance of this class.
>>> import math
>>> class NonCompliantDefault(Configurable):
Comment thread
Purg marked this conversation as resolved.
... def __init__(self, power_func=math.pow):
... self.power_func=power_func
... @classmethod
... def get_default_config(cls):
... default = super(NonCompliantDefault, cls).get_default_config()
... default['power_func'] = {'module': 'math',
... 'attribute': 'pow'}
... return default
>>> NonCompliantDefault.get_default_config() ==
... {'power_func': {'module': 'math', 'attribute': 'pow'}}
Comment on lines +79 to +80
Copy link
Copy Markdown
Member

@Purg Purg May 5, 2020

Choose a reason for hiding this comment

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

Failing doc-test: requires a slash at the end for the line continuation.

Suggested change
>>> NonCompliantDefault.get_default_config() ==
... {'power_func': {'module': 'math', 'attribute': 'pow'}}
>>> NonCompliantDefault.get_default_config() == \
... {'power_func': {'module': 'math', 'attribute': 'pow'}}

>>> import json
>>> json.dumps(NonCompliantDefault.get_default_config()) ==
... {"power_func": {"module": "math", "attribute": "pow"}}
Comment on lines +82 to +83
Copy link
Copy Markdown
Member

@Purg Purg May 5, 2020

Choose a reason for hiding this comment

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

Failing doc-test: requires a slash at the end for the line continuation.

Suggested change
>>> json.dumps(NonCompliantDefault.get_default_config()) ==
... {"power_func": {"module": "math", "attribute": "pow"}}
>>> json.dumps(NonCompliantDefault.get_default_config()) == \
... {"power_func": {"module": "math", "attribute": "pow"}}


:return: Default configuration dictionary for the class.
:rtype: dict
Expand Down Expand Up @@ -106,7 +123,7 @@ def from_config(cls, config_dict, merge_default=True):

This base method is adequate without modification when a class's
constructor argument types are JSON-compliant. If one or more are not,
however, this method then needs to be overridden in order to convert
this method then needs to be overridden in order to convert
from a JSON-compliant stand-in into the more complex object the
constructor requires. It is recommended that when complex types *are*
used they also inherit from the :class:`Configurable` in order to
Expand Down