-
-
Notifications
You must be signed in to change notification settings - Fork 387
field checks done too early causing error #1147
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
Comments
I concur, see also #1169 (comment) |
@sscherfke transformers are your labor of love – is this realistic without adding a whole abstraction layer over this? |
Not sure what you mean -- why isn't it sufficient to just move the order check after the field transformer has been applied? IIUC, field transformers can already do arbitrary rearranging, as long as the initial field order is valid. That said, why do we need the manual check in the first place? Python already gives us a pretty good exception when attempting to evaluate the incorrect code. Picking up the above example: def order_by_metadata(cls, fields):
fields.sort(key=lambda x: x.metadata["field_order"])
return fields
@define(field_transformer=order_by_metadata)
class A:
x: int = field(metadata={"field_order": 1})
y: int = field(metadata={"field_order": 0}, default=0) This yields the following error: Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/me/.local/lib/python3.11/site-packages/attr/_next_gen.py", line 144, in wrap
return do_it(cls, True)
^^^^^^^^^^^^^^^^
File "/home/me/.local/lib/python3.11/site-packages/attr/_next_gen.py", line 90, in do_it
return attrs(
^^^^^^
File "/home/me/.local/lib/python3.11/site-packages/attr/_make.py", line 1715, in attrs
return wrap(maybe_cls)
^^^^^^^^^^^^^^^
File "/home/me/.local/lib/python3.11/site-packages/attr/_make.py", line 1694, in wrap
builder.add_init()
File "/home/me/.local/lib/python3.11/site-packages/attr/_make.py", line 1090, in add_init
_make_init(
File "/home/me/.local/lib/python3.11/site-packages/attr/_make.py", line 2181, in _make_init
init = _make_method(
^^^^^^^^^^^^^
File "/home/me/.local/lib/python3.11/site-packages/attr/_make.py", line 345, in _make_method
_compile_and_eval(script, globs, locs, filename)
File "/home/me/.local/lib/python3.11/site-packages/attr/_make.py", line 317, in _compile_and_eval
bytecode = compile(script, filename, "exec")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<attrs generated init __main__.A>", line 1
def __init__(self, y=attr_dict['y'].default, x):
^
SyntaxError: non-default argument follows default argument |
* Perform attr order checks after field transformer. Fixes: #1147 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix assert style in test_hooks --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Hynek Schlawack <hs@ox.cx>
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [attrs](https://github.com/python-attrs/attrs) ([changelog](https://www.attrs.org/en/stable/changelog.html)) | project.dependencies | minor | `==25.1.0` -> `==25.2.0` | --- ### Release Notes <details> <summary>python-attrs/attrs (attrs)</summary> ### [`v25.2.0`](https://github.com/python-attrs/attrs/blob/HEAD/CHANGELOG.md#2520---2025-03-12) [Compare Source](python-attrs/attrs@25.1.0...25.2.0) ##### Changes - Checking mandatory vs non-mandatory attribute order is now performed after the field transformer, since the field transformer may change attributes and/or their order. [#​1147](python-attrs/attrs#1147) - `attrs.make_class()` now allows for Unicode class names. [#​1406](python-attrs/attrs#1406) - Speed up class creation by 30%-50% by compiling methods only once and using a variety of other techniques. [#​1407](python-attrs/attrs#1407) - The error message if an attribute has both an annotation and a type argument will now disclose *what* attribute seems to be the problem. [#​1410](python-attrs/attrs#1410) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xOTYuMCIsInVwZGF0ZWRJblZlciI6IjM5LjE5Ni4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL2RlcGVuZGVuY2llcyJdfQ==--> Reviewed-on: https://git.tainton.uk/repos/pypilot/pulls/316 Reviewed-by: Luke Tainton <luke@tainton.uk> Co-authored-by: Renovate [BOT] <renovate-bot@git.tainton.uk> Co-committed-by: Renovate [BOT] <renovate-bot@git.tainton.uk>
Attrs does some checks on fields like no defaulted fields coming before non-defaulted fields before running custom field transformers. This is a problem when a field transformer intends to modify the fields but gets an error before being able to apply their modification.
For example, this field transformer reorders fields so that in fact the defaulted field
x
comes after the non-defaulted fieldy
. But attrs errors before the transformer is ever called.I think these checks should be done after custom field transformers.
The text was updated successfully, but these errors were encountered: