Repository navigation
docs: add a Constants section to the Python code conventions - #75
Merged
Merged
Conversation
Module-level constants were everywhere in consumer code without a rule saying so, which left "should these be in a class or an enum?" open every time it came up. Record the two shapes that cover it — private to one module, and a namespace module holding a shared vocabulary — and the three properties that earn an Enum instead: exhaustive branching, iteration or membership, and a parameter typed "one of these". Absent all three an enum is churn, and a frozenset is the right container for a pure membership test. Also record the case the container choice does not fix: a constant spelled in two languages, where the duplicate fails silently.
The section restated a rule ruff already enforces - an UPPER_SNAKE local in a function is N806, and consumers select ALL - in a file whose own preamble says it holds the rules that rely on review. Drop it, and tighten the rest: the enum test reads as one sentence rather than a numbered list, and the two shapes lose their second explanations.
Three bare module names read as if they might be the repo's own, so the precedent they were carrying did not land. Say where they come from, and name a member of each rather than the module, which also shows the dotted access the bullet is about. Swap signal for string while here. signal.SIGTERM is a Signals IntEnum member, not a plain int - it is iterated and mapped back from number to name, so it is an example of the enum test two paragraphs down rather than of a plain namespace module.
The section was written in shorthand - shapes, shared vocabulary, authored-value surface, branched on exhaustively, churn - which a reader has to decode before they reach the rule underneath. Say the same three rules in ordinary words, and turn the enum test back into a list, since it is three separate questions and reads as one run-on sentence otherwise. No rule changed. The examples, the two cases and the cross-language warning are the same as before.
The two examples were shortened when the section was written, which disguised nothing - the names never carried an identity - and left them reading as module names nobody would write. Use the real ones, matching the Imports section above, which quotes its examples as they are.
The paragraph opened on the abstraction - a problem no container solves, a value written in two languages - and reached the actual case in a subclause, so a reader who had not already met the problem could not tell what it was describing. Put the submit button first, with both halves of the duplicate spelled out, and keep the rule for the end. Split the fix into its own paragraph while here, since it is the part a reader acts on.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Module-level constants were everywhere in consumer code with no rule saying so, which left "should these be in a class, or an enum?" open every time it came up. This records the answer.
coll_names.HTTP_LOG. Wrapping either in a class or a frozen dataclass buys nothing — a module is already a namespace, which is howerrno.ENOENTandstring.punctuationwork.Enum. Something must handle every value and fail to type-check when one is added; something loops over them or asks whether a value is one of them; or an argument should be typed as one of them. Absent all three it is work for nothing, and afrozensetis enough for a pure membership test.SAVE = "save"in Python,name="save"in a template — has nothing connecting the two copies, so a rename means Python silently stops finding the button. Hand the constants to the template engine instead.The section states only rules that rely on review, per the file's own preamble: an earlier draft argued that a function-local should not wear
UPPER_SNAKE, which is ruff's N806 underselect = ["ALL"], and that came back out.Test plan
awk 'length>80'over the file is clean — prose wraps at 80signal.SIGTERMturned out to be aSignalsIntEnum member, so it was swapped forstring.punctuationto keep all three examples plain constants