Skip to content
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

Non-immutable content of input_groups for WorkChains #919

Closed
giovannipizzi opened this issue Nov 15, 2017 · 2 comments
Closed

Non-immutable content of input_groups for WorkChains #919

giovannipizzi opened this issue Nov 15, 2017 · 2 comments
Assignees
Labels
topic/workflows type/question may redirect to mailinglist
Projects
Milestone

Comments

@giovannipizzi
Copy link
Member

If I do the following test, it fails as the content of the input group is not immutable and the next step will still see the changes.

This instead does not work with self.inputs as this is a FrozenDict. If this is the expected behaviour, this should be extended to groups.

As a side note, I think groups were implemented to solve the problem of use_ methods with an additional flag to be passed (e.g.: pseudos) and are now being (mis)used by many users just for organisational purposes, to group together inputs. Is this ok? (This also doesn't perform checks/validations, but I got from @sphuber that maybe in the next version this behaviour changed?)

    def test_immutable_input_groups(self):
        """
        Check that self.inputs.xxx are not modifiable where xxx is an input_group
        """
        test_class = self

        class Wf(WorkChain):
            @classmethod
            def define(cls, spec):
                super(Wf, cls).define(spec)
                spec.input_group('grp')
                # Try defining an invalid outline
                spec.outline(
                    cls.s1,
                    cls.s2,
                )

            def s1(self):
                grp = self.inputs.grp
                # I manipulate this 'inputs' dict - I expect that in the next step
                # self.inputs is not changed

                ## If we want the same behavior for group as for inputs, we want these exceptions to be raised
                ## Otherwise, if the dictionary can be changed, still we want the next test (in step s2) to work.
                with test_class.assertRaises(TypeError): # 'AttributesFrozendict' object does not support item assignment
                    grp['one'] = Int(3)
                with test_class.assertRaises(AttributeError): # AttributeError: 'AttributesFrozendict' object has no attribute 'pop'
                    grp.pop('two')
                with test_class.assertRaises(TypeError): # 'AttributesFrozendict' object does not support item assignment
                    grp['four'] = Int(4)

            def s2(self):
                grp = self.inputs.grp
                # a and b should be still there
                test_class.assertIn('one', grp)
                test_class.assertIn('two', grp)
                test_class.assertNotIn('four', grp)
                # The value should still be the original one, unmodified
                test_class.assertEquals(grp['one'].value, 1)

        x = Int(1)
        y = Int(2)
        run(Wf, grp={'one': x, 'two': y})
@giovannipizzi giovannipizzi added topic/workflows type/question may redirect to mailinglist labels Nov 15, 2017
giovannipizzi added a commit that referenced this issue Nov 15, 2017
(passes for `self.inputs`; fails for input_groups, see #919)
This is the first commit in a new branch to have some tests that
currently fail.
@giovannipizzi
Copy link
Member Author

I added a new branch (see referenced commit here above) with two new tests (one passing for self.inputs, one failing for input_groups

@sphuber sphuber added this to the v0.12.0 milestone Feb 2, 2018
@sphuber sphuber added this to To Do in Engine via automation Feb 2, 2018
@sphuber sphuber self-assigned this Feb 2, 2018
@sphuber sphuber moved this from To Do to In Progress in Engine Feb 2, 2018
@sphuber
Copy link
Contributor

sphuber commented Feb 5, 2018

This was addressed in issue #196 which was fixed in PR #1099

@sphuber sphuber closed this as completed Feb 5, 2018
Engine automation moved this from In Progress to Done Feb 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/workflows type/question may redirect to mailinglist
Projects
No open projects
Engine
  
Done
Development

No branches or pull requests

3 participants