Add Python standards #100

steve-dickinson · 2025-04-16T07:30:46Z

No description provided.

ben-sagar · 2025-04-16T14:46:30Z

This will need adding to the front page - not sure if it fits under the coding standards section or whether it should have its own one?

ben-sagar · 2025-04-16T14:47:43Z

docs/standards/python_coding_standards.md

+
+This document defines the internal development standards for building backend services in Python ([FastAPI](https://fastapi.tiangolo.com/tutorial/)) within Defra. It adheres strictly to the [UK Government Digital Service (GDS) Python style guide](https://gds-way.digital.cabinet-office.gov.uk/manuals/programming-languages/python/python.html) and [PEP 8](https://peps.python.org/pep-0008/), ensuring code is clear, consistent, and maintainable across all AI services (using tools like [LangChain](https://python.langchain.com/docs/introduction/), [LangGraph](https://langchain-ai.github.io/langgraph/tutorials/introduction/), etc.). All engineers must follow these conventions when writing Python code for backend services.
+
+**Note:** Python should **ONLY** be used for creating backend services related to AI or data.


I feel like everyone is using data - maybe this needs to say "data science" or something like that?

johnwatson484 · 2025-04-16T15:35:24Z

docs/standards/python_coding_standards.md

+
+**Note:** Python should **ONLY** be used for creating backend services related to AI or data.
+
+**Note:** Python should **NOT** be used for frontend development. The GDS Service Manual has information on using client-side JavaScript. For server-side JavaScript we use Node.js. The principles of [Progressive Enhancement](https://www.gov.uk/service-manual/technology/using-progressive-enhancement) should be used for the design and build of frontend digital services.


Agree with this point but do we need to go into details of client side JavaScript? Could we just say don't use it for frontend and instead use Node.js and reference our Node.js and JavaScript standards?

johnwatson484 · 2025-04-16T15:39:43Z

docs/standards/python_coding_standards.md

+**Note:** Python should **NOT** be used for frontend development. The GDS Service Manual has information on using client-side JavaScript. For server-side JavaScript we use Node.js. The principles of [Progressive Enhancement](https://www.gov.uk/service-manual/technology/using-progressive-enhancement) should be used for the design and build of frontend digital services.
+
+## Function Annotations and Typing Conventions
+- Use Type Hints for All Functions: All function and method definitions should include Python type annotations for parameters and return values (PEP 484 style). This improves readability and helps static analysis.


Is there a reason we should include them by default? We wouldn't say that JSDoc should be used for all JavaScript as it may be an unnecessary overhead so why would we be inconsistent with Python?

Is there a data science-y/AI-y reason?

Heh - have you tried reading Python code?! 🤣

johnwatson484 · 2025-04-16T15:40:39Z

docs/standards/python_coding_standards.md

+
+    _In this example, `item_id` is an int, `verbose` is a bool with default False, and the function returns a dictionary mapping strings to any type._
+
+- PEP 484 Syntax: Follow standard typing syntax for annotations:


Is there a linter that could enforce these conventions we could just mandate usage of? Similar to our StandardJS approach?

johnwatson484 · 2025-04-16T15:51:00Z

docs/standards/python_coding_standards.md

+    # Standard library imports
+    import json
+    import os
+    from pathlib import Path
+
+    # Third-party imports
+    import requests
+    from fastapi import APIRouter, HTTPException
+
+    # Local application imports
+    from app import config
+    from app.models import Item
+    from app.utils import calculate_score


This is pleasing to read for me as this is how I order module imports in Node.js. However, that is not a consistent approach taken across Defra and it's not something we mandate in the equivalent standards.

So I'm conflicted between whether this is a good idea because as a new language we've got more of an opportunity to be more opinionated from the beginning on lower level code choices, or if this is just going too low and will just lead to friction and debate for a hill I've got no passion to die on.

I agree, this does feel a bit like stepping over the bounds for a standard, but then the Python ecosystem is a bit different to that for Node (where everything is equal) and .NET (where everything by Microsoft is equal).

There's Python modules that are built in C and are a rock-solid underpinning of things like data science, which are maybe a bit different to some package that a hobbyist knocked together at the weekend.

But I'm not a Python expert, so perhaps I'm not aware of some specific value in mandating this?

johnwatson484 · 2025-04-16T15:53:15Z

docs/standards/python_coding_standards.md

+
+- Catch Specific Exceptions: When catching exceptions, catch the specific exception types you expect. Avoid bare `except`: clauses, as they catch system-exiting exceptions (`SystemExit`, `KeyboardInterrupt`) and obscure the cause of errors. If you truly need to catch any exception, use `except Exception:` (which excludes `SystemExit` and `KeyboardInterrupt`). Even then, consider logging the exception and re-raising if not handling it meaningfully.
+
+-Provide Informative Messages: When raising exceptions, include a clear message. For example: `raise ValueError("Threshold must be non-negative")` is more informative than just `raise ValueError`. In a FastAPI context, raising an `HTTPException(status_code, detail="...")` with a helpful message will return that info to API clients.


Suggested change

-Provide Informative Messages: When raising exceptions, include a clear message. For example: `raise ValueError("Threshold must be non-negative")` is more informative than just `raise ValueError`. In a FastAPI context, raising an `HTTPException(status_code, detail="...")` with a helpful message will return that info to API clients.

- Provide Informative Messages: When raising exceptions, include a clear message. For example: `raise ValueError("Threshold must be non-negative")` is more informative than just `raise ValueError`. In a FastAPI context, raising an `HTTPException(status_code, detail="...")` with a helpful message will return that info to API clients.

johnwatson484 · 2025-04-16T16:12:32Z

docs/standards/python_coding_standards.md

+
+- No Magic Numbers/Strings: Avoid using unexplained literal numbers or strings in code. If a value has significance, give it a name as a constant. For instance, instead of using `if len(data) > 1024: ...`, define `MAX_PAYLOAD_SIZE = 1024` and use `if len(data) > MAX_PAYLOAD_SIZE:`. This makes the code self-documenting.
+
+- Immutability: Treat constants as read-only. Do not modify constant values at runtime. If a configuration needs to change, it should be done via changing an environment variable or config file, not by reassigning an imported constant in code.


You can modify constants in Python? Urgh...

johnwatson484 · 2025-04-16T16:13:40Z

docs/standards/python_coding_standards.md

+    ```
+    In a FastAPI app, a common practice is to use Pydantic settings (BaseSettings) to define config classes that read from env variables. Either approach is fine, as long as sensitive info and environment-specific details are not hard-coded.
+
+- No Magic Numbers/Strings: Avoid using unexplained literal numbers or strings in code. If a value has significance, give it a name as a constant. For instance, instead of using `if len(data) > 1024: ...`, define `MAX_PAYLOAD_SIZE = 1024` and use `if len(data) > MAX_PAYLOAD_SIZE:`. This makes the code self-documenting.


This seems to be more general standards that Python specific and should apply across all languages.

Do we have a Python Sonar profile to enforce these?

johnwatson484 · 2025-04-16T16:27:38Z

My overall comment would be this is very opinionated and I'm conflicted about that. As a new language we've got more opportunity to be so than we did with JavaScript and C# as teams brought 10 years of diverse history with them before we attempted to standardise. Setting clear expectations from the beginning and giving teams less to debate over are never a bad thing.

But do worry if we're too opinionated on repo structures, import orders etc, then we'll encounter resistance and friction when we try to scale across teams and what's really important to us with Python may become lost in the detail.

Appreciate this comment doesn't really help, but thems me thoughts. 😂

steve-dickinson added 2 commits April 16, 2025 08:28

Add Python standards

7277367

spelling mistake

d148ef3

ben-sagar reviewed Apr 16, 2025

View reviewed changes

johnwatson484 reviewed Apr 16, 2025

View reviewed changes


		This document defines the internal development standards for building backend services in Python ([FastAPI](https://fastapi.tiangolo.com/tutorial/)) within Defra. It adheres strictly to the [UK Government Digital Service (GDS) Python style guide](https://gds-way.digital.cabinet-office.gov.uk/manuals/programming-languages/python/python.html) and [PEP 8](https://peps.python.org/pep-0008/), ensuring code is clear, consistent, and maintainable across all AI services (using tools like [LangChain](https://python.langchain.com/docs/introduction/), [LangGraph](https://langchain-ai.github.io/langgraph/tutorials/introduction/), etc.). All engineers must follow these conventions when writing Python code for backend services.

		Note: Python should ONLY be used for creating backend services related to AI or data.


		Note: Python should ONLY be used for creating backend services related to AI or data.

		Note: Python should NOT be used for frontend development. The GDS Service Manual has information on using client-side JavaScript. For server-side JavaScript we use Node.js. The principles of [Progressive Enhancement](https://www.gov.uk/service-manual/technology/using-progressive-enhancement) should be used for the design and build of frontend digital services.


		_In this example, `item_id` is an int, `verbose` is a bool with default False, and the function returns a dictionary mapping strings to any type._

		- PEP 484 Syntax: Follow standard typing syntax for annotations:


		- Catch Specific Exceptions: When catching exceptions, catch the specific exception types you expect. Avoid bare `except`: clauses, as they catch system-exiting exceptions (`SystemExit`, `KeyboardInterrupt`) and obscure the cause of errors. If you truly need to catch any exception, use `except Exception:` (which excludes `SystemExit` and `KeyboardInterrupt`). Even then, consider logging the exception and re-raising if not handling it meaningfully.

		-Provide Informative Messages: When raising exceptions, include a clear message. For example: `raise ValueError("Threshold must be non-negative")` is more informative than just `raise ValueError`. In a FastAPI context, raising an `HTTPException(status_code, detail="...")` with a helpful message will return that info to API clients.

	-Provide Informative Messages: When raising exceptions, include a clear message. For example: `raise ValueError("Threshold must be non-negative")` is more informative than just `raise ValueError`. In a FastAPI context, raising an `HTTPException(status_code, detail="...")` with a helpful message will return that info to API clients.
	- Provide Informative Messages: When raising exceptions, include a clear message. For example: `raise ValueError("Threshold must be non-negative")` is more informative than just `raise ValueError`. In a FastAPI context, raising an `HTTPException(status_code, detail="...")` with a helpful message will return that info to API clients.


		- No Magic Numbers/Strings: Avoid using unexplained literal numbers or strings in code. If a value has significance, give it a name as a constant. For instance, instead of using `if len(data) > 1024: ...`, define `MAX_PAYLOAD_SIZE = 1024` and use `if len(data) > MAX_PAYLOAD_SIZE:`. This makes the code self-documenting.

		- Immutability: Treat constants as read-only. Do not modify constant values at runtime. If a configuration needs to change, it should be done via changing an environment variable or config file, not by reassigning an imported constant in code.

Add Python standards #100

Are you sure you want to change the base?

Add Python standards #100

Uh oh!

Conversation

steve-dickinson commented Apr 16, 2025

Uh oh!

ben-sagar commented Apr 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnwatson484 commented Apr 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants