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

Use the un-underscored name if no alias attribute is there. #416

Merged
merged 2 commits into from
Sep 23, 2023

Conversation

jap
Copy link
Contributor

@jap jap commented Aug 14, 2023

This appears to be caused by a thinko in #391, this fixes it again.

@Tinche
Copy link
Member

Tinche commented Aug 14, 2023

Ah, cool. Can you add a changelog note? And a simple test would be great so it doesn't happen again.

@Tinche Tinche added the bug label Aug 14, 2023
@Tinche Tinche added this to the 23.2 milestone Aug 14, 2023
@jap
Copy link
Contributor Author

jap commented Aug 15, 2023

Actually -- given that cattrs depends on modern attrs and this is just compat code, let me simplify it!

@Tinche
Copy link
Member

Tinche commented Aug 30, 2023

Howdy, any progress on this? Trying to plan the next release.

@jap
Copy link
Contributor Author

jap commented Sep 1, 2023

Whoops, sorry - forgot about this PR, working on an updated version now!

This modern version of `attrs` provides the `alias` attribute on
fields that contains the name we should use here.

Also add a test to verify that structuring and unstructuring aliased
attributes (which can be triggered by prefixing an attribute with an
underscore) works as expected.
@jap jap marked this pull request as ready for review September 1, 2023 13:54
Copy link
Member

@Tinche Tinche left a comment

Choose a reason for hiding this comment

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

Looks good, left a comment

A class with an aliased attribute can be unstructured and structured.
"""

converter = Converter()
Copy link
Member

Choose a reason for hiding this comment

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

I think you should be testing this with a BaseConverter to hit the changed code path, the Converter uses different machinery.

@Tinche
Copy link
Member

Tinche commented Sep 4, 2023

Just a small note, I'll be away for the next 3 weeks on a family trip and I can continue reviewing this after I'm back!

@Tinche
Copy link
Member

Tinche commented Sep 23, 2023

I want to get the next release out so I fixed this myself. Thanks!

@Tinche Tinche merged commit c1e93e9 into python-attrs:main Sep 23, 2023
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants