Updated long infill method

[isaac-tfn]

Describe Changes

Fixes #116

Task Checklist

• [x] Have unittests been added (testing individual functions and their edge cases)
• [x] Have integration tests been added (if applicable, to test modules of functionality)
• [ ] Updated RELEASE.md to reflect changes in PR
• [ ] Have new dependencies been added?
  • [ ] Have they been added to either requirements.txt or requirements_dev.txt.
  • [ ] Have they been added to pyproject.toml

[isaac-tfn]

Needs linting and checking. Tests pass but currently not compatible with python 3.9

[BenTaylor-TfN]

Can you edit the description in your opening comment and explain what you've changed and why please? It will make reviewing much easier if I know what I'm keeping an eye out for

[isaac-tfn]

Can you edit the description in your opening comment and explain what you've changed and why please? It will make reviewing much easier if I know what I'm keeping an eye out for

I can't see an initial comment, will add to release.md?

[isaac-tfn]

merged main into this one and now suddenly everything is failing. I think a lot is because of some stuff not available in 3.9, but some stuff may be due to more recent versions of e.g. mypy

[MattBuckley-TfN]

fixed linting issues, are we all happy to merge this? @BenTaylor-TfN @isaac-tfn

[BenTaylor-TfN]

Can you summarise what's actually changed please? Save me reading the many changes and possibly still being confused

EDIT: Following a discussion I'm happy that the changes here are reflected in issue: #116

[BenTaylor-TfN]

Happy with the changes here. Would just like to see a once over on the outstanding comments before accepting the merge.

[MattBuckley-TfN]

Pylint is failing with the "Too many positional arguments" flag 17 times, is this something we want to fix or just up the limit to 10 (this will solve all but one of the issues).

The pylint docs suggest that we should force any arguments passed the first few as keyword-only, which does make sense to avoid confusion.

@isaac-tfn, @BenTaylor-TfN - what do you both think?

[BenTaylor-TfN]

What's changed in the code to make this appear now?

As far as forcing keyword arguments, I'm happy for us to go for this if it makes sense. My general rule of thumb on whether something should not be forced as a keyword is "Will this argument need to be passed in for 80% of the calls the this function?". If not, it can probably be a keyword one.

[MattBuckley-TfN]

What's changed in the code to make this appear now?

As far as forcing keyword arguments, I'm happy for us to go for this if it makes sense. My general rule of thumb on whether something should not be forced as a keyword is "Will this argument need to be passed in for 80% of the calls the this function?". If not, it can probably be a keyword one.

Looks like nothing, it worked for commit 436b967 with pylint v3.2.7 but doesn't work on v3.3.1. Running pylint=3.27 on the current repo doesn't show the issues.

[MattBuckley-TfN]

One caveat with forcing keyword arguments is that its technically a public API change so it might break stuff elsewhere

[BenTaylor-TfN]

What's changed in the code to make this appear now? As far as forcing keyword arguments, I'm happy for us to go for this if it makes sense. My general rule of thumb on whether something should not be forced as a keyword is "Will this argument need to be passed in for 80% of the calls the this function?". If not, it can probably be a keyword one.

Looks like nothing, it worked for commit 436b967 with pylint v3.2.7 but doesn't work on v3.3.1. Running pylint=3.27 on the current repo doesn't show the issues.

Looks like it's an update with the PyLint checks then in reality?

[BenTaylor-TfN]

I suggest we disable to new checker for now in PyLint and instead add a new issue to deal with this in another PR. This change is too far reaching to be a problem here.

[MattBuckley-TfN]

Yes, something they must have changed between v3.2 and v3.3. I'll limit pylint in requirements to 3.2