Looks like I made a duplicate of #845 , but that PR has more changes.

I completely understand where you're coming from, but it would be simpler to change the documentation, than to change the code.

My concern is that mutations to the dict, would now affect all instances' dicts. I don't know why a user would ever do that, but that's no reason to break their code.

More importantly, it would cause test pollution if that dict was mutated in testing (it would then affect that dict in other tests).

My first request is simply to rephrase the docstring, and any other references to it.

If there are still good reasons for this change, then my second request is to making the class variable name = MappingProxyType({ ... }) (a read only view from types) to make it obvious that mutations are not supported.

My reason for this change was to see the list of ReasonCode's to know which packetType and aName combination I was looking for, I was writing unit tests for some of the rejection logic and I needed to test MQTT server disconnects, since I was manually triggering the callbacks.

I agree changing the docs is easier, since I assume creating ReadonCode objects from outside is something that is rarely done.

Should I close this PR? I can take care of updating the docs referencing this part.

I think having ReasonCode.name visible is very useful. But if that mean's it's a class variable, it would just be even better if it was a read only one. This is what I'd do:

import functools
import warnings
from typing import Any
from types import MappingProxyType

from .packettypes import PacketTypes


@functools.total_ordering
class ReasonCode:
    """MQTT version 5.0 reason codes class.

    See ReasonCode.names for a list of possible numeric values along with their
    names and the packets to which they apply.

    """

    names = MappingProxyType({
        0: {
            "Success": [PacketTypes.CONNACK, PacketTypes.PUBACK, PacketTypes.PUBREC, PacketTypes.PUBREL, PacketTypes.PUBCOMP, PacketTypes.UNSUBACK, PacketTypes.AUTH],
            "Normal disconnection": [PacketTypes.DISCONNECT],
            "Granted QoS 0": [PacketTypes.SUBACK],
        },
        1: { ... },
        ...
    })
        

I tried to click the file in browser and make a suggestion it that way. But I ended up adding a PR on top of your PR (how are you two creating these suggestions - have I just not got permissions to do it?).

I can see that. I did not know about this MappingProxyType, cool.

Also to add a suggestion, when you have selected a line or a few lines on the code review, there a button that looks like a file and has a «±» in the middle. Click it and it adds the suggestion.

Nice one Pau - thankyou :). I was wondering how to do exactly that.

My reason for this change was to see the list of ReasonCode's to know which packetType and aName combination I was looking for, I was writing unit tests for some of the rejection logic and I needed to test MQTT server disconnects, since I was manually triggering the callbacks.

But therefore the option of updating documentation won't solve this problem ? Since you need a instance of the class to access those attributes.

Since the value are "constant", I do think they belong to class attributes (not instance attribute as they correctly do). But I agree that mutation could be an issue: if someone previously mutated the instance attributes, thinking it would be safe since it's instance attribute, our change shouldn't impact globally. So we should ensure user don't mutate by mistake the global values.

I never user MQTT v5 & Properties, but I don't known if the format exported by .names, .properties are fine or too complex (e.g. the "type" which is just a number, or the name which contains or not whitespace). But maybe it's not this PR that should rework this.

As minimal step, I suggest:

• Moving the information to class attribute, either with enforced read-only or clear enough documentation that mutation shouldn't be done (I'm okay if user is conscious about the risk of doing mutation and do the mutation)
• But to avoid any possible breaking change, copy the class attribute as instance attribute. Therefore mutation of instance is possible and don't affect class attribute.

What do you think about this suggestion ?

Making a mutable instance variable from a read only class variable sounds like the best of both worlds to me.

Changes applied. As a side note, IMO the ReasonCode class is weirdly implemented and could probably have a cleaner interface, but that's out of the scope of this PR.

Also, should I merge main to fix the lint checks?

The lint check is failing due to this mypy error:

src/paho/mqtt/reasoncodes.py:122: error: Incompatible types in assignment (expression has type "dict[int, dict[str, list[int]]]", variable has type "MappingProxyType[int, dict[str, list[int]]]")  [assignment]

I'll take a look. Hopefully it's a straightforward annotation.

Alr thanks @JamesParrott , it looks like shadowing self.names is messing with mypy.