Trailing comma is not deleted in a one-line dict literal

[Wim-De-Clercq]

Describe the style change
When a specific dict with callables gets the key looked up and the callable called in one go, the formatting is both uglier than the original and also invalid for flake8.

Examples in the current Black style
Before formatting:

value = "1"
key = "integer"

x = {
    "string": str,
    "integer": int,
}[key](value)

After formatting:

value = "1"
key = "integer"

x = {"string": str, "integer": int,}[
    key
](value)

This is not valid for flake8, and looks worse than what it was.

Desired style

value = "1"
key = "integer"

x = {
    "string": str,
    "integer": int,
}[key](value)

Or perhaps

value = "1"
key = "integer"

x = {"string": str, "integer": int}[key](value)

[JelleZijlstra]

This looks like we're mishandling the magic trailing comma.

[haxsaw]

I'm also getting trailing comma problems in other circumstances. Here is some code pre-black, formatted by PyCharm:

Deployment(apiVersion='apps/v1beta1', kind='Deployment',
           metadata=ObjectMeta(name='nginx-app-2', labels={'app': 'nginx'}),
           spec=DeploymentSpec(selector=LabelSelector(matchLabels={'app': 'nginx'}),
                               template=PodTemplateSpec(
                                   metadata=ObjectMeta(labels={'app': 'nginx'}),
                                   spec=PodSpec(containers=[
                                       Container(name='nginx', image='nginx:1.15.4',
                                                 ports=[
                                                     ContainerPort(containerPort=80)])])),
                               replicas=3))

And here's how black leaves it:

Deployment(
    apiVersion="apps/v1beta1",
    kind="Deployment",
    metadata=ObjectMeta(name="nginx-app-2", labels={"app": "nginx"}),
    spec=DeploymentSpec(
        selector=LabelSelector(matchLabels={"app": "nginx"}),
        template=PodTemplateSpec(
            metadata=ObjectMeta(labels={"app": "nginx"}),
            spec=PodSpec(
                containers=[
                    Container(
                        name="nginx",
                        image="nginx:1.15.4",
                        ports=[ContainerPort(containerPort=80)],
                    )
                ]
            ),
        ),
        replicas=3,
    ),
)

To be fair, the black code is correct and is much nicer to read, but those unnecessary trailing commas are unfortunate.

[Wim-De-Clercq]

The trailing commas you have there are intentional to have a cleaner history in git.
If you have

settings = {
   "one": 1
}

For commit 1 and for another issue you have to add a new setting in commit 2:

settings = {
   "one": 1,
   "two": 2
}

Then your history of the line "one": 1 will show as latest commit number 2 in the history.
Unless you use the trailing commas.

[haxsaw]

If that's the case, it doesn't seem to be an evenly applied rule. For instance from the fragment I published, why doesn't:

                containers=[
                    Container(
                        name="nginx",
                        image="nginx:1.15.4",
                        ports=[ContainerPort(containerPort=80)],
                    )
                ]

have a comma after the ')' that comes before the closing ']'? The last parameter to Container has a comma after it, but the Container() creation itself doesn't. For this rule, shouldn't it be every dict or list?

Additionally, for this larger fragment:

    spec=DeploymentSpec(
        selector=LabelSelector(matchLabels={"app": "nginx"}),
        template=PodTemplateSpec(
            metadata=ObjectMeta(labels={"app": "nginx"}),
            spec=PodSpec(
                containers=[
                    Container(
                        name="nginx",
                        image="nginx:1.15.4",
                        ports=[ContainerPort(containerPort=80)],
                    )
                ]
            ),
        ),
        replicas=3,
    ),

Putting a comma after the 'replicas=3' makes the assumption that there are other params that might be added that would benefit from the rule you cited above. While true in this case, there's no way for Black to know whether params have be exhausted or not, and so it adds a bit of visual noise for a small git optimization that may never be taken advantage of.

It strikes me that the trade-off to consider here is whether reading an extra line in a git diff happens more or less than reading and understanding code with a superfluous comma. I would expect that reading the code happens more than the git diff, but that's just me.

[hauntsaninja]

The trailing comma issue here looks fixed (tested with 23.3). Black still doesn't format this ideally, IMO, but splitting of subscript is a subject for another issue: https://black.vercel.app/?version=stable&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4ADKAH9dAD2IimZxl1N_Wmp8V60GiUhqmIgp3NEfJZ80rLoYJDsyKQz74j89jeSqxnkhE91U8Gnoag9Ohh4vVkLYu2UqoYKwVHpthCus4KjeAtYsDgBXLvDv1eTBRUMKkkw1Mt6SRA7UK_YEMfX7YY09tH7uaWbo9K9E3WV9NfWo3TKK8QAAAFy5mzuRwyOEAAGbAcsBAADdGBopscRn-wIAAAAABFla