Skip to content

Conversation

dmontagu
Copy link
Contributor

Closes #6745

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 21, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: f203806
Status: ✅  Deploy successful!
Preview URL: https://436da25d.pydantic-docs2.pages.dev
Branch Preview URL: https://error-for-invalid-field-name.pydantic-docs2.pages.dev

View logs

elif isinstance(value, FieldInfo) and not is_valid_field_name(var_name):
suggested_name = var_name.lstrip('_') or 'my_field' # don't suggest '' for all-underscore name
raise NameError(
f'Fields must not use names with leading underscores;'
Copy link
Member

@hramezani hramezani Jul 21, 2023

Choose a reason for hiding this comment

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

Suggested change
f'Fields must not use names with leading underscores;'
f'Field() must not use names with leading underscores;'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We say Private attributes above, which I was trying to mirror here with the description of what it was, rather than the function used to create it. I actually think it's less confusing like this because people might not realize that calling Field() signals to pydantic that that thing should be a field, which is what I want to emphasize.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, I've edited my suggestion

Copy link
Member

@hramezani hramezani left a comment

Choose a reason for hiding this comment

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

Other than minor suggestions LGTM!

@dmontagu dmontagu merged commit df1a67e into main Jul 21, 2023
@dmontagu dmontagu deleted the error-for-invalid-field-names branch July 21, 2023 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Computed Fields that depend on private fields don't work properly
2 participants