Skip to content

Commit

Permalink
Do not ignore type_string in constructor of Group (#3935)
Browse files Browse the repository at this point in the history
Doing so would actually break backwards compatibility. Code that creates
groups with explicit custom type strings, would no longer be able to
query for them as the type string was silently converted to `core`. Even
though accepting the passed `type_string` will cause warnings when
loading them from the database, that is preferable then breaking
existing code.
  • Loading branch information
sphuber authored Apr 15, 2020
1 parent 789fdee commit 0e2a00c
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
7 changes: 6 additions & 1 deletion aiida/orm/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,14 @@ def __init__(self, label=None, user=None, description='', type_string=None, back
raise ValueError('Group label must be provided')

if type_string is not None:
message = '`type_string` is deprecated because it is determined automatically, using default `core`'
message = '`type_string` is deprecated because it is determined automatically'
warnings.warn(message) # pylint: disable=no-member

# If `type_string` is explicitly defined, override automatically determined `self._type_string`. This is
# necessary for backwards compatibility.
if type_string is not None:
self._type_string = type_string

type_string = self._type_string

backend = backend or get_manager().get_backend()
Expand Down
6 changes: 3 additions & 3 deletions tests/orm/test_autogroups.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def test_get_or_create(self):
)

# I create a group with a large integer suffix (9)
AutoGroup(label='{}_9'.format(label_prefix), type_string='auto.run').store()
AutoGroup(label='{}_9'.format(label_prefix)).store()
# The next autogroup should become number 10
autogroup = Autogroup()
autogroup.set_group_label_prefix(label_prefix)
Expand All @@ -69,7 +69,7 @@ def test_get_or_create(self):
)

# I create a group with a non-integer suffix (15a), it should be ignored
AutoGroup(label='{}_15b'.format(label_prefix), type_string='auto.run').store()
AutoGroup(label='{}_15b'.format(label_prefix)).store()
# The next autogroup should become number 11
autogroup = Autogroup()
autogroup.set_group_label_prefix(label_prefix)
Expand All @@ -86,7 +86,7 @@ def test_get_or_create_invalid_prefix(self):
label_prefix = 'new_test_prefix_TestAutogroup'
# I create a group with the same prefix, but followed by non-underscore
# characters. These should be ignored in the logic.
AutoGroup(label='{}xx'.format(label_prefix), type_string='auto.run').store()
AutoGroup(label='{}xx'.format(label_prefix)).store()

# Check that there are no groups to begin with
queryb = QueryBuilder().append(AutoGroup, filters={'label': label_prefix})
Expand Down
27 changes: 27 additions & 0 deletions tests/orm/test_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,33 @@ def test_loading_unregistered():

assert isinstance(loaded, orm.Group)

@staticmethod
def test_explicit_type_string():
"""Test that passing explicit `type_string` to `Group` constructor is still possible despite being deprecated.
Both constructing a group while passing explicit `type_string` as well as loading a group with unregistered
type string should emit a warning, but it should be possible.
"""
type_string = 'data.potcar' # An unregistered custom type string

with pytest.warns(UserWarning):
group = orm.Group(label='group', type_string=type_string)

group.store()
assert group.type_string == type_string

with pytest.warns(UserWarning):
loaded = orm.Group.get(label=group.label, type_string=type_string)

assert isinstance(loaded, orm.Group)
assert loaded.pk == group.pk
assert loaded.type_string == group.type_string

queried = orm.QueryBuilder().append(orm.Group, filters={'id': group.pk, 'type_string': type_string}).one()[0]
assert isinstance(queried, orm.Group)
assert queried.pk == group.pk
assert queried.type_string == group.type_string

@staticmethod
def test_querying():
"""Test querying for groups with and without subclassing."""
Expand Down

0 comments on commit 0e2a00c

Please sign in to comment.