Remove Optional from a few places

[antonymilne]

Description

Following a discussion in #190 (comment) about whether or not we should have Optional in our field type hints e.g. for Navigation, I've made a PR to implement what I think should be our preferred solution.

I agree with everything @huong-li-nguyen said in #190 (comment), which means that whether or not we specify Optional has no effect at runtime - it's purely of interest for mypy.

The problem we have is that mypy (correctly) interprets navigation: Optional[Navigation] = None as giving navigation the type Union[None, Navigation]. In reality if it's None then we populate it straight away in a validator, so we know that it can't be None, but mypy doesn't know this unless you tell it explicitly.

There are various ways to perform this type narrowing e.g. assert self.navigation is not None, using cast or worse just putting in type: ignores. All of these would work but litter the code quite a bit. See e.g. the assert solution in https://mypy.readthedocs.io/en/stable/kinds_of_types.html#optional-types-and-the-none-type.

On the other hand, if we set navigation: Navigation = None then mypy interprets the type (probably incorrectly tbh...) as Navigation with no option of None. The catch here is that we need to do type: ignore[assignment] or mypy will complain at this point that you're assigning None to something that can't be None.

So the choice is between doing something a bit naughty at point of assignment vs. doing something annoying every time the optional field is consumed. Since the field is defined once but consumed multiple times, overall I think it's best to do the naughty thing at point of assignment so that all consuming code can be cleaned up.

Following this and #189 we now have what I would deem an acceptably small number of type: ignores in our codebase, although when we do bump to pydantic v2 I suspect this will need to tackled again...

I deliberately haven't changed e.g. value: Optional[List[float]] = None in RangeSlider. The reason for this is:

• these examples (I think?) actually have None as a genuine value rather than a sentinel value that gets replaced at runtime
• I know these fields have been discussed many times before but I can't remember the details so didn't want to make any changes I wasn't sure of
• they don't cause any type: ignores or other annoyances currently, and since the reason for this change is purely to appease type checking it doesn't seem worth spending more time on
  ... but in theory if we think it's better to change these also then we could do so.

Checklist

• I have not referenced individuals, products or companies in any commits, directly or indirectly
• I have not added data or restricted code in any commits, directly or indirectly
• I have updated the docstring of any public function/class/model changed
• I have added tests to cover my changes (if applicable)

Types of changes

• Docs/refactoring (non-breaking change which improves codebase)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Notice

• I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
  • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

[huong-li-nguyen]

Thanks for double-checking again! 🚀

What you wrote makes a lot of sense and I agree that this is probably still a bit naughty, but better than the other alternative 👍

[maxschulz-COL]

I like this change, and I think the reasoning is sound although of course in a perfect world this would not be necessary