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

Prefer dict literal over dict constructor #7690

Closed
hofrob opened this issue Oct 30, 2022 · 3 comments · Fixed by #7691
Closed

Prefer dict literal over dict constructor #7690

hofrob opened this issue Oct 30, 2022 · 3 comments · Fixed by #7691
Labels
Enhancement ✨ Improvement to a component
Milestone

Comments

@hofrob
Copy link
Contributor

hofrob commented Oct 30, 2022

Current problem

Constructing dicts using the constructor should be avoided. It has no advantages and may be a bit slower.

Invalid code

# construct dict using keywords
dict(foo="bar")
# construct dict using a dict
dict(**boo)

Valid code

# dict literal
{"foo": "bar"}

# casting values to a dict
dict(some_method())

# methods that are not the builtin `dict`
def dict(baz):
    ...

dict(1)

Desired solution

Create a message that finds and highlights invalid code from above. Maybe as an extension of R1735 use-dict-literal.

Additional context

No response

@hofrob hofrob added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Oct 30, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Proposal 📨 Needs decision 🔒 Needs a decision before implemention or rejection and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Oct 30, 2022
@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Work in progress and removed Proposal 📨 Needs decision 🔒 Needs a decision before implemention or rejection labels Oct 30, 2022
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.16.0 milestone Oct 30, 2022
@nickdrozd
Copy link
Contributor

I just came across this new check (from running latest in Astroid). It seems awfully opinionated to me. Is it the official stance of Pylint that this syntax is bad? Is it old or deprecated or something? The official documentation doesn't say much about it, just that "when the keys are simple strings, it is sometimes easier to specify pairs using keyword arguments". So one reason you might want it is if you don't want extra strings floating around.

It's certainly not a widely used way to make dictionaries, but I don't see why it should be recommended against. Is it really slower?

@hofrob
Copy link
Contributor Author

hofrob commented Nov 30, 2022

https://github.com/PyCQA/pylint/blob/main/doc/data/messages/u/use-dict-literal/details.rst

If this was a feature where the devs decided that one version looks better than the other and style was the only consideration, I'd agree that it is opinionated. But this was not the case here at all.

Literals are faster. And faster is better when there's so little difference in style (my personal opinion). It would even make sense to expand this rule to sets and lists (maybe this is already working or in the works).

@nickdrozd
Copy link
Contributor

Okay, I'm agreed then on the other constructors as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants