Skip to content

ENH: Add tab completion for node and interface inputs properties #2735

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

Merged
merged 8 commits into from
Oct 17, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
10 changes: 9 additions & 1 deletion nipype/interfaces/base/specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,11 @@ def __repr__(self):
""" Return a well-formatted representation of the traits """
outstr = []
for name, value in sorted(self.trait_get().items()):
outstr.append('%s = %s' % (name, value))
if not name == '__all__':
outstr.append('%s = %s' % (name, value))
Copy link
Member

Choose a reason for hiding this comment

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

Is this still necessary, with __all__ becoming a property?

return '\n{}\n'.format('\n'.join(outstr))


Copy link
Member

Choose a reason for hiding this comment

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

We should remove this line.

def _generate_handlers(self):
"""Find all traits with the 'xor' metadata and attach an event
handler to them.
Expand Down Expand Up @@ -309,6 +311,11 @@ def _get_sorteddict(self,
out = objekt
return out

@property
def __all__(self):
return [k for k,v in self.items() if not k == "__all__"]
Copy link
Member

Choose a reason for hiding this comment

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

Given that items() provides a dict keyed by self.copyable_trait_names(), and you only need keys, I think you can just make this:

@property
def __all__(self):
    return self.copyable_trait_names()

You'll need to change the BET tests to either sort or check set equality, but this should work.



Copy link
Member

Choose a reason for hiding this comment

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

Too many blank lines.


class TraitedSpec(BaseTraitedSpec):
""" Create a subclass with strict traits.
Expand Down Expand Up @@ -351,6 +358,7 @@ def __deepcopy__(self, memo):
# clone twice
dup = self.clone_traits(memo=memo)
dup.trait_set(**dup_dict)
self.__all__ = self.class_editable_traits()
return dup


Expand Down
48 changes: 48 additions & 0 deletions nipype/interfaces/base/tests/test_specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
from ....utils.filemanip import split_filename
from ... import base as nib
from ...base import traits, Undefined
from ....interfaces import fsl
from ...utility.wrappers import Function
from ....pipeline import Node


standard_library.install_aliases()

Expand Down Expand Up @@ -47,6 +51,20 @@ class spec(nib.TraitedSpec):
assert infields.__repr__() == '\nfoo = 1\ngoo = 0.0\n'


def test_TraitedSpec_tab_completion():
bet_nd = Node(fsl.BET(), name = 'bet')
bet_interface = fsl.BET()
bet_inputs = bet_nd.inputs.class_editable_traits()
bet_outputs = bet_nd.outputs.class_editable_traits()

# Check __all__ for bet node and interface inputs
assert bet_nd.inputs.__all__ == bet_inputs
assert bet_interface.inputs.__all__ == bet_inputs

# Check __all__ for bet node outputs
assert bet_nd.outputs.__all__ == bet_outputs


@pytest.mark.skip
def test_TraitedSpec_dynamic():
from pickle import dumps, loads
Expand All @@ -63,6 +81,36 @@ def test_TraitedSpec_dynamic():
assign_a_again


def test_DynamicTraitedSpec_tab_completion():
def extract_func(list_out):
return list_out[0]

# Define interface
func_interface = Function(input_names=["list_out"],
output_names=["out_file","another_file"],
function=extract_func)
# Define node
list_extract = Node(Function(
input_names=["list_out"],output_names=["out_file"],
function=extract_func), name="list_extract")

# Check __all__ for interface inputs
expected_input = sorted(list_extract.inputs.editable_traits())
assert(sorted(func_interface.inputs.__all__) == expected_input)

# Check __all__ for node inputs
assert(sorted(list_extract.inputs.__all__) == expected_input)

# Check __all__ for node outputs
expected_output = sorted(list_extract.outputs.editable_traits())
assert(sorted(list_extract.outputs.__all__) == expected_output)

# Add trait and retest
list_extract.outputs.add_trait("added_out_trait","val")
Copy link
Member

@effigies effigies Oct 16, 2018

Choose a reason for hiding this comment

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

The problem is that the output spec is actually instantiated in IOBase._outputs(), which calls Function._add_output_traits, which only will add things in self._output_names. So to do what you want here, you would need to do:

list_extract._interface._output_names.append('added_out_trait')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed. Thank you. I don't think I would have got that.

expected_output = sorted(['added_out_trait',*expected_output])
Copy link
Member

Choose a reason for hiding this comment

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

Python 2.7, 3.4 don't like the [*x] syntax. How about making expected_output a set above, moving to set comparisons, and changing this to:

expected_output.add('added_out_trait')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice. Thanks for the tip. done

assert(sorted(list_extract.outputs.__all__) == expected_output)


def test_TraitedSpec_logic():
class spec3(nib.TraitedSpec):
_xor_inputs = ('foo', 'bar')
Expand Down