use `context = Context(...)` or `state = State(...)` instead of `context = State(...)`

[jlapeyre]

It could decrease cognitive load a bit if we did this.

It would be similar to what we do here https://github.com/Qiskit/qiskit-qasm3-import/blob/246e999573c0d7a2162b779d9525b8b5b79c3cf0/src/qiskit_qasm3_import/expression.py#L297

and here https://github.com/Qiskit/qiskit-qasm3-import/blob/246e999573c0d7a2162b779d9525b8b5b79c3cf0/src/qiskit_qasm3_import/converter.py#L344

[jakelishman]

Where are we writing context = State(...)? I don't see any cases in the code.

The parameter is called context because it's defined by the ABC visitor, and in general the context of an arbitrary visit can be anything. The object I used in ConvertVisitor is called State because it represents the current mutable state of the conversion, but I could equally have chosen to only pass the symbol table through context and keep the circuit in the class instance, or a different QASMVisitor might be doing something totally different and want to pass some external immutable context object unrelated to the state.

[jlapeyre]

You're correct. Nothing like context = State(...) appears. I can't locate whatever it was that prompted me to open this issue. If I find it later I'll open a new one.

[jlapeyre]

I recall why I opened this. In converter.py, the string "context: State" occurs 29 times in parameter lists. Variables of type State are consistently named context.

In the comment above, I think you referring to this https://github.com/Qiskit/qiskit-qasm3-import/blob/246e999573c0d7a2162b779d9525b8b5b79c3cf0/src/qiskit_qasm3_import/converter.py#L173

which subclasses this

class QASMVisitor(Generic[T]):
    """...
    We added the context argument to the visit method. It allows the visitor
    to hold temporary state while visiting the nodes.
    ...
    """

    def visit(self, node: QASMNode, context: Optional[T] = None):
         ...

So context is the name of a parameter of parametric type. The doc string refers to both "state", a general term, as well as "context", a narrower category that implies what the state's meaning or function is. I think using the more precise term "context" for the parameter was a good choice.

I understand that we could pass any existing structure that holds the context we want. But in fact we don't. We define a tailor made class for the purpose. But with State we used the more general term. That's not such a big deal in itself. But it seems to be gratuitously different from the more natural choice Context. I don't see any disadvantage to using Context rather than State. It would be very quick edit. And it reduces cognitive load by a small amount.

On the other hand, it's a rather minor point. It's certainly not a problem if you want to leave it as it is.

[jakelishman]

I think we agree on the logic, but have a difference of opinion on the stylistic detail - to me, State is the more specific word. Since it's primarily just a stylistic point, I'd rather just leave things in place.