Specifying a feature: OxRSE Training

Introduction

In the previous module, we looked at how to specify a problem in a real-world codebase so that we can direct coding agents to help us to address it. For a well-written issue, a lot of the time the reporter already knows what is broken (the traceback/error message) and what the expected behaviour should be, and our task is to make sure the issue says it clearly enough and write an appropriate prompt to direct the coding agents to handle it.

While a lot of principles still apply, implementing a feature request with a coding agent requires you to consider more than fixing a bug. Nobody encounters an actual error so the coding agent does not have something to 'fix', and what the expected behaviour should be is often a decision someone has to make. Adding a feature with coding agents often results in subtle changes in other parts of the code as coding agents tend to apply a change too broadly, or simply misunderstand your feature. This module is about how to use the power of coding agents to implement feature requests effectively.

Considerations before implementing a feature

Coding agents will happily implement any feature you or your collaborators can think of ('say the word and I will...'). With the power of coding agents, code can be produced quickly and it is very tempting to implement something just because you can. It is very satisfying to see a lot of output being 'written' into your codebase with additional functionalities as it gives us a sense of achievement and productivity.

However, the first question to ask yourself before implementing anything should always be: should I implement this feature? Below are some things to think about before implementing a feature:

do I or the users really need this feature?
what do I gain or lose without this feature?
is this feature solving something for the majority of the users?
is the additional complexity proportional to the potential improvement?
who is going to maintain this feature?
what are the potential negative impacts on the users?

As the saying goes, 'the best code is no code at all'. Every additional line of code will introduce maintenance cost, potential bugs, and particularly when working with coding agents, cognitive load on a scale humans are just not able to deal with.

A case study with scikit-learn SLEP025

SLEPs (Scikit-Learn Enhancement Proposals) are scikit-learn's process for a feature that changes shared and public behaviour, similar to PEPs in Python. A condensed paraphrase of SLEP025 is included below with status and outcome removed.

For the sake of the training, please refrain from reading the original proposal and all associated discussions until the end of this section.

SLEP025: Losing Accuracy in Scikit-Learn Score

Abstract:

This SLEP proposes to rectify the default score method for scikit-learn classifiers. Currently, classifier.score(X, y) applies accuracy, which has well known deficiencies (it is not a strictly proper scoring rule, and hard-codes a 50% probability threshold). This SLEP changes the default.

Motivation:

Accuracy is the most used metric for classifiers via score(), largely through blind default use rather than a conscious choice, and it has misled users. The situation calls for a correction.

Solution:

Introduce a scoring keyword to score(). Default stays "accuracy" for classifiers, "r2" for regressors.
Deprecate the default "accuracy" for classifiers.
After the deprecation period, change the default to "d2_brier_score".

Open questions: how long the deprecation period should be (proposal: 4 minor releases instead of the usual 2, given the impact), and whether the new default should be D2 Brier score or a model-specific objective.

Backward compatibility:

Feasible within scikit-learn's usual deprecation strategy.

Alternatives:

Remove score entirely: forces an active choice, but is a disruptive, unmarketable API break.

Keep status quo: no disruption, but perpetuates a known bad default and signals the project cannot correct it.

Would you vote for it?

Read the above proposal and imagine you are one of the maintainers. Would you vote for or against the proposal? Why?

The vote closed on 2026-02-24 with the proposal rejected by roughly 8 votes to 3, and you can read all the comments in this PR. Most maintainers agreed that accuracy is a poor default and several supported the first two steps in 'Solution'. The concerns are mostly about the suggested defaults: some raised issues with the D2 Brier score, some saw more harm than good in changing a default that so much downstream code relies on and suggested an opt-in route instead.

The author's closing comment was that 'there is no good default metric'. It was also not very clear how and where the deprecation should be raised, for example whether cross_validate or classifier.score should warn, and this is one of the most important things to settle as this involves changing its default silently in the future. People using score would suddenly need to handle a deprecation warning, then potentially change their code to accommodate this upstream change if they want to keep the existing behaviour. A change that impacts a public interface with subtle behaviour change should be carefully considered before it is made.

Interface, boundary and acceptance

After deciding to go ahead with a feature and use coding agents for assistance, three areas should be considered when specifying a feature to the coding agents:

Interface: what should exist afterwards, such as additional parameters, the values they accept, a change in default, what are the return values etc.
Boundary: what should not be changed.
Acceptance: checks to make sure a plausible but wrong implementation would fail.

Suppose you have a function fit_peak(y, x) which returns the peak centre and there is a feature request that its uncertainty should also be returned. Should the uncertainty be returned as a second value (i.e. a tuple) or using an object to encapsulate it? Should a parameter such as return_uncertainty be added and default to False? This is the interface you should think about.

Current users of the function write peak_centre = fit_peak(y, x), and if now a tuple or a custom object is returned, how would this impact the current users? Should this feature break existing usage or aim for backwards compatibility instead? This is the boundary you should set.

To make sure the uncertainty is correctly calculated, knowing that a value is returned is not enough. Does the value change if the same peak is fitted twice with different levels of noise? Is the uncertainty smaller when the noise is lower? Are there edge cases in the calculation of uncertainty that give you NaN? This is the acceptance you should check.

If you miss out anything about the interface, boundary or acceptance when implementing a feature with coding agents, capable coding agents are very willing to guess, or will offer suggestions if you ask. The guess or suggestion they make often sounds very reasonable, especially with well-versed justification, and this is exactly where the danger is. If you do not have a good understanding of the feature, you may be swayed into implementing something that is subtly wrong.

Be as explicit as possible about what you would like to implement, as coding agents do not know exactly your context and you should always be the one who makes the decision and takes ownership of it. Taking credit when everything is fine but quickly pointing the finger at coding agents when something is broken is not a responsible use of this powerful technology.

A case study with SciPy issue 11841

For the sake of the training, please refrain from reading the original issues/PR links and related discussion online until the end of this section.

The issue requested a feature to handle missing values in a fit:

Occasionally I need to fit data that's incomplete (missing values) or perform a fitting task over a grid of 3-D data, i.e. fitting M points for every NxN element, where one of the elements needs to be masked out or ignored.

It may be a good feature to have a flag (or flags) to curve_fit that tell it to ignore NaNs in an array being fit and return NaN when there are no floats/data to fit (i.e. all nans).

I've seen documented that when you drop the check for nan/infs in curve_fit you can silently get nonsensical results. It seems a flag here would provide a way to define the behavior you'd like when nans/inf are present.

It may be helpful to take a look at the documentation of curve_fit in the last release before the feature was added. It fits a model function to data by non-linear least squares and it refuses input containing NaN, which is what data with missing values usually looks like.

First attempt

Identify the interface, boundary and acceptance

Imagine you are going to use a coding agent to implement the above feature request and based solely on the above text and its signature, identify the interface, boundary, acceptance and what cannot be known from the issue alone.

Interface: a flag that makes curve_fit ignore NaN instead of raising
Boundary: every existing call should behave exactly as before
Acceptance: fitting data with NaN with the above flag on should give the same results as fitting the data with the corresponding NaN absent; data with all NaN should return NaN.

However, a lot of things are not clear from the feature request itself, such as:

the name of the flag, whether there is a convention or not in the codebase.
what the flag should take, whether it is a boolean, a string, an enum etc.
if the flag takes a string or an enum, what would be the options?
it seems there are documented ways to deal with NaN or any other invalid values, what are they and which one should I choose?
the boundary above assumes the existing behaviour is kept for backwards compatibility, but this is a decision and it is not clear whether we should keep it.
curve_fit can take 3 arrays: xdata, ydata and sigma, and does the flag apply to all 3 arrays?
how should the new flag interact with check_finite?

What actually happened

Below is a brief summary of the timeline when this feature was being implemented, and it settled a lot of decisions that would have to be made if one directed a coding agent to implement it.

Apr 2020: the feature request is made and maintainers agree it is reasonable.
Sep 2022 to Jan 2023: PR 16961 is submitted to implement the feature and some decisions are settled during the review:
- the flag should be named nan_policy following SciPy's convention.
- the flag should accept strings with 'omit' and 'raise' as options.
- the flag does not take 'propagate' as an option although it is SciPy's convention as it is not clear what it means in this context.
- the flag defaults to None which keeps the existing behaviour.
- the default of the existing check_finite flag is changed from True to None: it is treated as True when nan_policy is not given, and as False when it is, so an explicit check_finite=True still takes priority.
Oct 2024: a user reports a bug in gh-21772 saying it does not correctly handle nan_policy='omit' with sigma. 'omit' drops the NaN entries in xdata and ydata, but not from sigma, so the shape is incorrect as sigma should have a one-to-one correspondence with ydata. The newly added tests do not catch this as no sigma was passed.
Nov to Dec 2024: PR 21918 fixes the above bug.

Identify them again

Now that we know much more with the benefit of hindsight, identify the interface, boundary and acceptance, imagining you would direct a coding agent to implement the feature.

Interface: a flag nan_policy that accepts 'omit', 'raise' and None, default to None to keep backwards compatibility; the default of check_finite becomes None, which means True when nan_policy is not specified and False when it is.
Boundary: every existing call should behave exactly as before, backwards compatibility should be kept.
Acceptance: fitting data with NaN with nan_policy='omit' should give the same results as with the corresponding NaN entries absent; it should raise when there is NaN in the data with nan_policy='raise'; it should raise when any other string is passed to nan_policy such as 'propagate'; passing nan_policy='omit' together with an explicit check_finite=True should still raise on NaN; all 3 arrays xdata, ydata and sigma should be tested with or without NaN for all the existing fitting methods.

Note that the original request to return NaN when all data are NaN was never implemented and this is a decision which is absent from the original request.

Real-world feature implementation rarely happens in isolation, and it is your responsibility to engage with users or maintainers to find out what the feature should look like exactly, or at least make sensible decisions and consider the possibility of subtle behaviour or edge cases, to minimise introducing potential bugs. Modern coding agents rarely make trivial mistakes such as syntax errors or typos, so any unexpected behaviour is far more likely a gap in your specification. If you just give the original issue to a coding agent and prompt it with 'Implement this', you risk having a broken feature in your codebase.

A case study with xarray PR 9407

The specification below is a version reconstructed by FeatBench from PR 9407, which added support for byte attributes. The reconstructed version was used as one of the examples to evaluate the capability of coding agents to implement something when it was clearly specified. Treat it as what a specification could look like once someone has already worked out what it should be.

netCDF is a file format for saving labelled and multi-dimensional data. xarray can write it via several backends, two of which matter here: netcdf4 and h5netcdf.

I want to be able to save datasets with byte attributes when using xarray's netCDF export functionality, while ensuring compatibility with different storage engines. Specifically, I need:

Support for bytes as dataset attributes alongside existing supported types (strings, numbers, arrays, lists/tuples, and numpy numbers)
Special validation when using the h5netcdf engine to ensure byte attributes are compatible with its limitations
Automatic detection and handling of incompatible byte data for h5netcdf with clear error messages

When I save datasets using the h5netcdf engine, I want the system to:

Check if any attribute values are bytes
Verify that these bytes can be decoded as UTF-8 strings without errors
Ensure the byte data contains no null characters (zero bytes)
Provide helpful error messages suggesting alternative engines if the bytes are incompatible
Allow valid UTF-8 encoded bytes without null characters to be saved successfully

For other engines like netcdf4, I want byte attributes to be accepted without these additional validations, maintaining the existing behavior.

I also want the validation to properly handle the existing invalid_netcdf parameter behavior, ensuring that numpy boolean types are only allowed when both invalid_netcdf is True and the engine is h5netcdf, while maintaining all existing validation rules for other data types.

The system should validate attributes throughout the entire dataset structure, including both dataset-level attributes and those on individual data arrays, providing consistent error messages that clearly identify which attribute name and value caused any validation failures.

Identify interface, boundary and acceptance

Identify its interface, boundary and acceptance, with a particular focus on boundary.

Interface: bytes is added to the attribute types it already accepts and this applies to every backend.
Boundary: the extra byte checks (decoded as UTF-8 string, no null bytes) run only for h5netcdf but not netcdf4. The existing rule that numpy booleans are allowed only when invalid_netcdf is True on h5netcdf must also stay unchanged.
Acceptance: valid UTF-8 bytes save successfully on both backends; a byte object with a null byte or invalid UTF-8 is accepted by netcdf4 but h5netcdf raises with a message naming the attribute and suggesting netcdf4; a numpy boolean with invalid_netcdf=False is still rejected on h5netcdf.

Coding agents look for similarity in the codebase and tend to apply the same change everywhere it looks relevant. 'Support byte attributes' sounds like a straightforward change, but this also adds a restriction that must apply to h5netcdf only. If a coding agent applies the UTF-8 and null-byte checks to every backend, the feature silently drops something that is working before: netcdf4, which can store those bytes, now rejects them. The h5netcdf test still passes because it only checks that an error is raised, and it takes a human who knows the backends to notice this boundary.

With the netcdf4 backend, a bytes attribute is written as-is as NC_CHAR (netCDF's 8-bit character type) and can be read back as a string, with undecodable bytes replaced and anything after a null byte dropped. With h5netcdf, the attribute is written as an HDF5 variable-length UTF-8 string, which must decode as UTF-8 and cannot contain a null byte.

Conclusion

Decide yourself and/or with maintainers/users whether the feature should exist at all before specifying and implementing it.
Interface is about what must exist afterwards (new parameters, accepted values, defaults, returns).
Boundary states what must not be changed and where the new behaviour must not apply. Coding agents tend to change everything that looks relevant and it relies on you to specify it correctly.
Acceptance checks both what must now succeed and what must still be rejected. Merely passing tests and not raising an error do not mean it is really working.
Changing a public behaviour requires a clear migration/deprecation plan for users who never asked for it.