Skip to content

Commit

Permalink
Make ArrayStore fields be valid identifiers (#398)
Browse files Browse the repository at this point in the history
## Description

<!-- Provide a brief description of the PR's purpose here. -->

Although we mostly use the field names as dict keys, which can take on
any value, it may be useful in the future to be able to use fields as
identifiers. For example, if we are using kwargs, it makes sense to be
able to pass in a field name as `f(field1=[...])`, which is not possible
if `field1` is an invalid identifier like `field foo` (note the space).

## TODO

<!-- Notable points that this PR has either accomplished or will
accomplish. -->

- [x] Add check
- [x] Add test

## Questions

<!-- Any concerns or points of confusion? -->

## Status

- [x] I have read the guidelines in

[CONTRIBUTING.md](https://github.com/icaros-usc/pyribs/blob/master/CONTRIBUTING.md)
- [x] I have formatted my code using `yapf`
- [x] I have tested my code by running `pytest`
- [x] I have linted my code with `pylint`
- [x] I have added a one-line description of my change to the changelog
in
      `HISTORY.md`
- [x] This PR is ready to go
  • Loading branch information
btjanaka committed Nov 3, 2023
1 parent 22d40ce commit 1038582
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 6 deletions.
2 changes: 1 addition & 1 deletion HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
({pr}`397`)
- **Backwards-incompatible:** Rename `measure_*` columns to `measures_*` in
`as_pandas` ({pr}`396`)
- Add ArrayStore data structure ({pr}`395`)
- Add ArrayStore data structure ({pr}`395`, {pr}`398`)
- Add GradientOperatorEmitter to support OMG-MEGA and OG-MAP-Elites ({pr}`348`)

#### Improvements
Expand Down
14 changes: 10 additions & 4 deletions ribs/archives/_array_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ class ArrayStore:
dtype)``. For instance, ``{"objective": ((), np.float32),
"measures": ((10,), np.float32)}`` will create an "objective" field
with shape ``(capacity,)`` and a "measures" field with shape
``(capacity, 10)``.
``(capacity, 10)``. Note that field names must be valid Python
identifiers.
capacity (int): Total possible entries in the store.
Attributes:
Expand All @@ -101,8 +102,10 @@ class ArrayStore:
_fields (dict): Holds all the arrays with their data.
Raises:
ValueError: One of the fields in ``field_desc`` has an invalid name
(currently, "index" is the only invalid name).
ValueError: One of the fields in ``field_desc`` has a reserved name
(currently, "index" is the only reserved name).
ValueError: One of the fields in ``field_desc`` has a name that is not a
valid Python identifier.
"""

def __init__(self, field_desc, capacity):
Expand All @@ -117,7 +120,10 @@ def __init__(self, field_desc, capacity):
self._fields = {}
for name, (field_shape, dtype) in field_desc.items():
if name == "index":
raise ValueError(f"`{name}` is an invalid field name.")
raise ValueError(f"`{name}` is a reserved field name.")
if not name.isidentifier():
raise ValueError(
f"Field names must be valid identifiers: `{name}`")

if isinstance(field_shape, (int, np.integer)):
field_shape = (field_shape,)
Expand Down
13 changes: 12 additions & 1 deletion tests/archives/array_store_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
# pylint: disable = redefined-outer-name


def test_init_invalid_field():
def test_init_reserved_field():
with pytest.raises(ValueError):
ArrayStore(
{
Expand All @@ -17,6 +17,17 @@ def test_init_invalid_field():
)


def test_init_invalid_field():
with pytest.raises(ValueError):
ArrayStore(
{
# The space makes this an invalid identifier.
"foo bar": ((), np.float32),
},
10,
)


@pytest.mark.parametrize("shape", [((), (2,), (10,)), ((), 2, 10)],
ids=["tuple", "int"])
def test_init(shape):
Expand Down

0 comments on commit 1038582

Please sign in to comment.