Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
23 changes: 8 additions & 15 deletions src/apispec/ext/marshmallow/field_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,21 +242,14 @@ def field2choices(
"""
attributes = {}

comparable = [
validator.comparable
for validator in field.validators
if hasattr(validator, "comparable")
]
if comparable:
attributes["enum"] = comparable
else:
choices = [
OrderedSet(validator.choices)
for validator in field.validators
if hasattr(validator, "choices")
]
if choices:
attributes["enum"] = list(functools.reduce(operator.and_, choices))
choices = []

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've been wondering why we don't use a set here and update it in the loop.

This is probably due to a slight performance advantage doing it this way.

for validator in field.validators:
if hasattr(validator, "choices"):
choices.append(OrderedSet(validator.choices))
elif hasattr(validator, "comparable"):
choices.append(OrderedSet([validator.comparable]))
if choices:
attributes["enum"] = list(functools.reduce(operator.and_, choices))

if field.allow_none:
enum = attributes.get("enum")
Expand Down
57 changes: 56 additions & 1 deletion tests/test_ext_marshmallow_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,62 @@ def test_field_with_choices_multiple(spec_fixture):
]
)
res = spec_fixture.openapi.field2property(field)
assert set(res["enum"]) == {"brian", "john"}
assert res["enum"] == ["brian", "john"]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess the use of set here was intentional as we don't want to ensure the order.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done — reverted to set() comparison.



def test_field_with_choices_multiple_non_intersecting(spec_fixture):

@lafrech lafrech Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd write oneof instead of choices in the test name.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Renamed.

field = fields.Str(
validate=[
validate.OneOf(["freddie", "brian"]),
validate.OneOf(["john", "roger"]),
]
)
res = spec_fixture.openapi.field2property(field)
assert res["enum"] == []


def test_field_with_choices_and_equal(spec_fixture):
field = fields.Str(
validate=[
validate.OneOf(["freddie", "brian", "john"]),
validate.Equal("brian"),
]
)
res = spec_fixture.openapi.field2property(field)
assert res["enum"] == ["brian"]


def test_field_with_choices_and_equal_non_intersecting(spec_fixture):
field = fields.Str(
validate=[
validate.OneOf(["freddie", "brian"]),
validate.Equal("john"),
]
)
res = spec_fixture.openapi.field2property(field)
assert res["enum"] == []


def test_field_with_multiple_equal_matching(spec_fixture):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the whole thing could be factorized in three tests:

  • multiple oneof
  • multiple equal
  • oneof and equal

using parametrization to pass inputs and expected output.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Factorized into three parametrized tests: multiple_oneof, multiple_equal, oneof_and_equal.

field = fields.Str(
validate=[
validate.Equal("brian"),
validate.Equal("brian"),
]
)
res = spec_fixture.openapi.field2property(field)
assert res["enum"] == ["brian"]


def test_field_with_multiple_equal_conflicting(spec_fixture):
field = fields.Str(
validate=[
validate.Equal("freddie"),
validate.Equal("brian"),
]
)
res = spec_fixture.openapi.field2property(field)
assert res["enum"] == []


def test_field_with_additional_metadata(spec_fixture):
Expand Down