search

gchq / coreax

A library for coreset algorithms, written in Jax for fast execution and GPU support.
Apache License 2.0
15 stars 1 forks source link

Replace | with Union until 3.9 no longer supported #636

Closed db091756 closed 3 weeks ago

db091756 commented 1 month ago

What's the new feature?

Python 3.10 introduced the shorthand X | Y to replace Union[X, Y], until we stop supporting 3.9. we should use Union.

What value does this add?

to use | in versions less than 3.10 we have to use from __future__ import annotations, but all this does is rewrite every type hint like str | int to the string "str | int". This has some issues, see https://stackoverflow.com/questions/66734640/any-downsides-to-using-from-future-import-annotations-everywhere. By explicitly using the "old" Union syntax we avoid this problem.

Is there an alternative you've considered?

No response

Additional context

No response

db091756 commented 1 month ago

Very easy fix, will do when #581 is complete

tp832944 commented 4 weeks ago

An alternative is to manually insert quotes around annotations where required. I don't care enough to specify one approach or the other. If going for Union, do make use of Optional too.

I agree dropping from __future__ import annotations is a very good idea. See #366, #378 and #515, which will all need updating.

Note that there is a from __future__ import annotations in coreset.py, and I can't see why it's still required.