Quality Assurance of Analytical Code playbook

[nhsbsa-mawil]

This guidance is around the assurance of analytical code and low code workflows. These standards and guidance do not replace existing government, NHS, or BSA guidance but give the specifics of how we (should) write and assure code used in analysis across multiple languages and tools including R, Python, SQL, Alteryx, etc.

Workshops were carried out in May 2023 to understand current practices and documentation across Data teams.

This Discussion area will be used by the working group contributing to the guidance to draft ideas and as a collaborative space.

Topics

• modular code/workflows
• readable code/workflows
• project and workflow structure
• documentation
• version control
• configuration
• data management
• peer review
• testing

Possible additional topics:

• logging
• end-to-end journey (path to prod)

Proposed timeline

1. draft of playbook go-live Q3 2023/24 (alpha)
2. comms about playbook (who should be using and how)
3. engage with users to keep developing and improving content
4. creation of tool and language specific examples
5. integrate with existing development standards and documentation across teams such as that in confluence
6. develop KPIs to track usage of playbook and adoption of practices across teams

Resources

Code assurance workshop summary
Code assurance SMT slides

[nhsbsa-mawil]

Working group - 15 Nov 2023

Attendees

• Matthew Wilson
• Pat Turner
• Dan Thompson
• Steven Buckley
• Kayoung Goffe
• Mark Gordon

Agenda

• intros
• purpose of the group and playbook
• fitting into wider DDaT
• playbook ontology
• timelines
• tools to use

Decisions and actions

• it was decided to develop the guidance with the existing DDaT Playbook and use the Discussions place to bring together the group and draft the guidance.
• all members to check they have GitHub account or create one
• we would tackle 'low hanging fruit' topics initially to get the ball rolling. Modular/readable/structured code first topic(s) to be written.

• We have a deadline of end of December 2023 to pull together drafts.
• members agreed being able to dedicate half a day's work every 2 weeks to playbook

[nhsbsa-mawil]

Modular code and workflows

Pre-requisite reading

AF Duck book: Modular code
NHSBSA DDaT playbook: Development - Coding

What is modular code?

Complex analysis projects and workflows can often lead to a lot of code being written with some or a lot of that code performing repetitive tasks. Breaking down your code and workflow into smaller reusable chunks to can help you maintain your project and share components across a series of analyses. Tools such as R, Python, SQL, and Alteryx provide methods to create containerised code such as modules, classes, functions and macros.

Why should I do this?

Writing your code in a modular way provides a range of benefits:

• prevents long, complex walls of code that are hard to understand
• makes sharing code with others (and your future self) easier
• speeds up the peer review process
• makes testing of the code (and its outputs) easier

How do we do it then?

Writing modular code or developing modular workflows can differ depending on the your tool of choice, the needs of the project or piece of work, the intended customer, or the expected users of the code in the future. However, a simple hierarchy of actions to take are:

1. split complex code and workflows in multiple (ordered) scripts or smaller workflows
2. use a code notebook to organise your analysis
3. write re-usable code as functions
4. organise related functions into modules
5. organise and document modules as packages

Let's dive into how we can do some of these.

Split your code into multiple scripts

Monolithic code is a big no-no. A large and complex piece of code that runs into hundreds, if not thousands, of lines is unwieldy, difficult to read, debug and maintain, and pretty mean to pass off for peer review on a Friday afternoon. Splitting your monolithic code into ordered scripts can help others (and yourself) understand the steps you've taken and the reasons why.

DRAFT NOTE: intention here is to provide examples for the various tools that teams use e.g. R, Python, SQL, Alteryx, VBA (also need to be conscious of future data platform)

For example, in R a large project could be split into:

project
-- 01_data_build.R
-- 02_data_clean.R
-- 03_analysis.R
-- 04_visualisation.R

these scripts can now be run in their intended order and splits out the parts of your code into related areas, making peer review easier.

Using scripts doesn't inherently make your code reproducible or reusable, but are a good first step towards better and higher quality code.

Do

• split your code up
• group related code together
• organise your scripts so you and others know how to run it
• make your scripts self contained and be able to ran from a command line

Don't

• write monolithic code
• split up your code too much - 100 scripts with 10 lines of code in are just as hard to digest
• write overly complex code where simpler solutions might exist

Use a code notebook

Services such as Jupyter notebooks or R Markdown allow you write code, commentary and visualisations alongside each other. They can be incredibly useful, and individual workbooks can be a good first step towards modularising your code for simple projects and workflows and for doing initial data exploration. However, they can be difficult to use with further practices for producing high quality code. For example:

• they are inherently difficult to review and audit using version control
• code cells can be ran out of order, reducing reproducibility
• not easy to test functions written in the notebook

How will this be enforced?

[nhsbsa-mawil]

Working group - 13 Dec 2023

Attendees

• Matthew Wilson
• Pat Turner
• Steven Buckley
• Kayoung Goffe
• Graham Platten
• Brad Cullen
• Mark Gordon

Agenda

• review/carry forward actions
• review modular code guidance (structure and content)
• AOB

Summary

• point raised around not advertising unsupported/unsecure tools - remove VBA from guidance
• enforcement of standards understood to be long journey, first step is getting guidance out there and members of the group acting as champions for those standards
• map out the different routes to live for each area - compare and find common patterns
• point raised about in-depth examples for each tool and language, including turning bad practice into good. Good idea to create specific repos that analysts can clone and play around with
• is playbook about learning/CPD or standards (what the practice looks like at the BSA)? Agreed that need to be clear in our articles, split them into what we do and then how to do it. But question remains on if the learning should be part of the playbook
• important that we use a consistent voice with the rest of the playbook with so many different contributors. Pat has raised with content design and taking this forward as a broader point
• identify consistent standards across DDaT and apply them here e.g. version control/peer review

Actions

• members to check if they have a GitHub account and create one if not
• topics to be dished out to members to start contributing the 'what we do' in the area
• draft guidance for chosen topics to be written for meeting on 24 Jan 2024
• next meeting on 27 Dec 2023 to be cancelled due to leave

Topics

Matthew - Modular code + configuration
Brad - Version Control (AF guidance / BSA standards)
Kayoung - readable code (AF guidance / BSA naming conventions)
Steven - peer review (AF guidance / BSA guidance)
Graham - documentation (AF guidance / BSA guidance)
Mark - project structure (AF guidance)

[nhsbsa-kaygo]

Readble code

Pre-requisite reading

AF Duck book: Reable code
NHSBSA DDaT playbook: Naming conventions

What is reable code?

Readable code is written with clarity and simplicity, making it easy for others to understand and maintain. It prioritises the reader's experience over the convenience of writing code.

Why should we write reable code?

• Enhances team collaboration
• Facilitates easier debugging and maintenance
• Encourages best coding practices
• Speeds up the onboading process for new team memebers

How do we write reable code?

• Use clear and descriptive namees for variables and functions
• Write small, reusable function that do one thing well
• Comment wisely to explain the 'why', not the 'what'
• Adhere to a consistent coding style
• Regularly refactor to improve code clarity

By following these guidelines, we create a codebase that is easier to read, understand, and contribute to, ultimately leading to a more efficient and effective development process. For more detailed insights, please refer to the recommended pre-requisite reading.

How will this be enforced?

[nhsbsa-stbuc]

Peer Review

Pre-requisite reading

AF Duck Book: Peer review
NHSBSA DDaT Playbook: Development - Peer Review

What is peer review?

Peer review is a process where a second person will review a colleagues code before any code is finalised or used to produce output.

Why should I do this?

Although peer reviews can reduce potential bugs being missed, the peer review process is not simply a case of trying to catch errors.

There are a number of benefits that to using a peer review approach, including:

• Improving quality of code
  • As part of the peer review process code will be reviewed to ensure that it is readable and understandable
• Sharing knowledge
  • Review code can improve the knowledge of both the original code author and the reviewer
• Optimising code performance
  • As part of the review process, different approaches may be found that could improve the code
• Find potential mistakes in the code
  • Although peer review is not a replacement for testing, it can find potential bugs or issues in the code

How do we do it then?

When using version control tools such as Git, peer review should be part of the branching and pull request process (see DDaT Playbook. Ideally a pull request should not be merged without the code being reviewed by somebody else.

Using version control tools the basic steps for a peer review are (DRAFT NOTE: do we need more details or is this repeating DDaT playbook):

1. Code author submits merge request relating for branch used to develop code
2. Reviewer reviews the code and adds comments for any potential issues, including the following areas:
   • Is the code readable and understandable?
   • Is the code tested, and does the code do what it is designed for?
   • Could the problem have been approached in a different way?
3. Author reviews comments and amends code as needed
   • Comments can and should be challenged if there is any disagreement to agree a resolution between the author and reviewer
4. Reviewer approves amended code
   • Checking all issues have been resolved
5. Branch merged

Do

• Try and incorporate peer review during the development process to get feedback during development
• Communicate clearly to ensure both author and reviewer understand and are in agreement at each stage
• Document any issues and decisions
• Try running all code to ensure it works as designed

Don't

• Simply glance over code and accept if it "looks right"
• Submit huge pull requests with multiple/large scripts where the reviewer has had not sight of the work previously
• Blindly accept comments from a reviewer without making sure you agree with the comment
• Be tempted to skip the review process to progress work quickly

It would be strongly recommended that version control tools are implemented and used for all pieces of work (DRAFT NOTE: link to playbook section on version control). However, where version control tools may not be in place, a peer review process would still be recommended, and notes should still be captured around any feedback and amendments raised during a review process.

Pair programming

Instead of having the code writing and peer review as two distinct phases, pair programming combines these into a single step with two or more people actively working on the same piece of code at the same time. Effective pair programming will involve one person writing the code whilst the other(s) supply real time feedback on the code being written.

The benefits of pair programming are that feedback can be provided as the code is being written, suggesting improvements or identifying/fixing issues as they appear. Additionally, pair programming can be a good method to share knowledge and experience between individuals.

Pair programming may require more resource during the development stage but at the benefit in a reduce of resource needed for a review stage.

For pair programming to be effective there are guidelines that should be followed:

• Rotate roles often

  • Everyone should take turns writing and reviewing which could mean switching roles every 15-30 minutes. This will ensure everyone gets experience from both sides of the process and will solidify the knowledge of the code being written.

• Document large decisions

  • Pair programming allows real-time discussion and decision making but where big decisions in terms of approach have been made these should still be documented for anybody who may use the code in future.

• Be vocal and ask questions

  • Pair programming is a collaborative process and should not just involve one person watching another code. The person writing the code should be talking through what they are writing, and the reviewer should be supplying feedback and asking for clarification if there are parts that they do not understand. Everybody who has been involved writing the code should have full understanding as if they had written it all themselves.

How will peer review be enforced?

DRAFT NOTE: need to determine how best this can be enforced

[nhsbsa-alijo]

Picking up this old thread with a few questions/comments:

1. Where in the digital playbook did we imagine the code quality assurance living?
   1. At the top level next to design, delivery, etc?
   2. Within the coding sections?
   3. Or somewhere else?
2. Do we have a set of core / must-have requirements for alpha release? Is it just the topics with draft pages above?
3. Every page has a section on 'how will X be enforced?' This feels like the wrong emphasis:
   1. Even if we wanted to (and I doubt we would) we would never have the resources to check everyone's code for adherence
   2. The word 'enforced' makes the playbook feel more like a set of rules than a resource that is supposed to help people. I think we need to look more at how adherence can be encouraged, tracked and understood so we can spot ways we can drive adherence
4. Some ideas for tracking adherence
   1. Badges on repos (e.g. peer-reviewed, test coverage, readability /10, reproducibility, etc)
   2. Central spreadsheet / github project board with status of each project (e.g. Red/Amber/Green or Bronze/Silver/Gold) for how well aligned to the guidance it is

[nhsbsa-alijo]

On point 3: maybe we should be shouting out / highlighting successful projects on e.g. the D&A community, internal message boards, LinkedIn posts etc?

[nhsbsa-mawil]

Hi Alistair,

1. a new "Data" section is being created for all data related playbooks to live in such as data vis etc. however, for prototyping and rapid development we can use the solution that has been used for the data science and quality analysis playbooks with a view to link out, or lift and shift content when appropriate.
2. The core requirements driven by potential users most likely don't align with what is most needed from our view point on the topics that are most priority. My own objective is to have high level guidance for all topics listed above as requirement for Alpha. As discussed, we could also align the requirements for Alpha with "Baseline" RAP.
3. I agree, enforced is definitely the wrong term. Originally it was aimed at "how to do we make sure people use this", but we don't have the resource to check people's adoption - nor should we. We need to look at the carrot rather than the stick for this e.g. we will give teams dedicated help if they keep us in the loop with how they're doing on adoption.
4. I like the both of those ideas, putting the emphasis on self reporting by teams.

building on conversations, also putting the project board for this here
https://github.com/orgs/nhsbsa-data-analytics/projects/10/views/1