basedpyright icon indicating copy to clipboard operation
basedpyright copied to clipboard

Optional Special Forms: They're Not Deprecated

Open fny opened this issue 1 year ago • 2 comments

Currently both Optional and Union are marked as deprecated as of 3.10, but the official Python documentation as of 3.12 does not mention these deprecations. Similarly, the PEPs do not explicitly deprecate these either.

export const deprecatedSpecialForms = new Map<string, DeprecatedForm>([
    ['Optional', { version: pythonVersion3_10, fullName: 'typing.Optional', replacementText: '| None' }],
    ['Union', { version: pythonVersion3_10, fullName: 'typing.Union', replacementText: '|' }],
]);

While Optional is a matter of taste, there are certain cases where Union is required. Assume there's a "B" type defined lower in a file. The following hints fail with the pipe syntax, but succeed with Union.

class A:
    pass
C =  A | "B" # => TypeError: unsupported operand type(s) for |: 'type' and 'str
C = Union[A, "B"] # => Works!

class B:
    pass

In my opinion, these special forms should be either removed altogether or separated from deprecateTypingAliases and moved into deprecateSpecialTypingAliases.

fny avatar Oct 02 '24 15:10 fny

the decision to mark these as deprecated came from pyright, and while the word "deprecated" isn't used in the docs, they do recommend using the new syntax over the old syntax so i believe they should be treated as deprecated. you can disable this behavior by setting deprecateTypingAliases to false

previous discussions:

  • Union - https://github.com/DetachHead/basedpyright/issues/242#issuecomment-2030679879
  • Optional - https://github.com/DetachHead/basedpyright/issues/456#issuecomment-2195640596

DetachHead avatar Oct 02 '24 21:10 DetachHead

While Optional is a matter of taste, there are certain cases where Union is required. Assume there's a "B" type defined lower in a file. The following hints fail with the pipe syntax, but succeed with Union.

class A:
    pass
C =  A | "B" # => TypeError: unsupported operand type(s) for |: 'type' and 'str
C = Union[A, "B"] # => Works!

class B:
    pass

basedpyright already handles this scenario gracefully:

C = int | "A"  # Union syntax cannot be used with string operand; use quotes around entire expression  (reportGeneralTypeIssues)

class A:
    ...

playground

KotlinIsland avatar Oct 08 '24 00:10 KotlinIsland

closing for the reasons mentioned in the above comments. if you disagree feel free to comment and we can re-open for further discussion if needed

DetachHead avatar Oct 27 '24 10:10 DetachHead

While Optional is a matter of taste, there are certain cases where Union is required. Assume there's a "B" type defined lower in a file. The following hints fail with the pipe syntax, but succeed with Union.

class A:
    pass
C =  A | "B" # => TypeError: unsupported operand type(s) for |: 'type' and 'str
C = Union[A, "B"] # => Works!

class B:
    pass

basedpyright already handles this scenario gracefully:

C = int | "A" # Union syntax cannot be used with string operand; use quotes around entire expression (reportGeneralTypeIssues)

class A: ...

playground

Basedpyright seem to stop emitting an error since 1.20.0 (1.19.1 do emit one), is that intentional?

https://basedpyright.com/?pyrightVersion=1.20.0&typeCheckingMode=all&code=MIAgvCCWB2AuIB8QCICCyBQGDGAbAhgM6EioBcGIVIAdHRkA

Glinte avatar Oct 26 '25 06:10 Glinte

hmm looks like that error is only reported if enableExperimentalFeatures is enabled (the default for that setting was changed from true to false in 1.20.0). i don't see why this functionality would need to be considered experimental, so i assume it's a bug. i've opened a separate issue for this: #1608

DetachHead avatar Oct 27 '25 09:10 DetachHead

@DetachHead As many have mentioned on the corresponding pyright ticket, Optional and Union are not deprecated and the documentation does not actually recommend using the X | None syntax; it is just mentioned as an alternative.

As pointed out by commenters on the pyright issue, something being deprecated (i.e. might be removed soon, so your code might potentially stop working in the future) and just having an alternative is not the same. Developers should definitely attend to the first, while the second is a matter of preference. Please reconsider relegating the deprecation to a lower priority warning (if even).

DavidNemeskey avatar Nov 11 '25 10:11 DavidNemeskey

@DavidNemeskey you can disable this rule if you don't like it

what issue do you have with using the newer better version?

KotlinIsland avatar Nov 11 '25 10:11 KotlinIsland

@KotlinIsland Do you mean deprecateTypingAliases? Doesn't that also disable warning for actually deprecated constructs, such List?

I don't see how X | None is better than Optional[X]. In any case, I think that with formal validation, preferences should take a second seat to facts; i.e. Optional and Union are not deprecated.

DavidNemeskey avatar Nov 11 '25 23:11 DavidNemeskey

I don't see how X | None is better than Optional[X]

Optional[T] is just an alias to T | None, adding a layer of obscurity imo. if we had union notation from the get-go there never would have been an Optional. and in my opinion, "Optional" is not obvious in it's meaning or semantics, probably why the docs have to elaborate this. also, X | None doesn't require an import

the documentation does not actually recommend using the X | None syntax; it is just mentioned as an alternative.

for Union the docs say:

Using that shorthand is recommended.

- https://docs.python.org/3/library/typing.html#typing.Union

for Optional, it's not so explicit in the docs, but as @DetachHead has pointed out: https://discuss.python.org/t/clarification-for-pep-604-is-foo-int-none-to-replace-all-use-of-foo-optional-int/26945/5

Do you mean deprecateTypingAliases? Doesn't that also disable warning for actually deprecated constructs, such List?

ah yes. my bad, sorry. but could you please describe how this diagnostic negativly impacts your usage and experience with basedpyright? and explain how disabling it, while not ideal, isn't an acceptable fix?

KotlinIsland avatar Nov 12 '25 06:11 KotlinIsland

There are situations with using forward references where X | None is not possible while Optional[X] is, especially when the type annotation needs to be evaluated at runtime. The 3.12 type syntax does allow you to do X | None even for forward references if you do type X = "a.b.X", but TypeAliasType has very poor, often special cased, runtime support with third party libraries that is different from how they handle forward references/strings (e.g. sqlalchemy)

Disabling deprecateTypingAliases negatively impacts me by making it impossible to see errors on actually deprecated types, which is very often written by AI.

Additionally, @leycec pointed out in https://github.com/microsoft/pyright/issues/9793#issuecomment-3448051180 that the typing constructs are self-caching

@leycec Do you mind chiming in here too?

Glinte avatar Nov 12 '25 06:11 Glinte

There are situations with using forward references where X | None is not possible while Optional[X] is

Disabling deprecateTypingAliases negatively impacts me by making it impossible to see errors on actually deprecated types, which is very often written by AI.

thank you for explaining. while we can now see that there are valid use cases for this constructs, we are not interested supporting them as Ruff already has excellent support for these cases with configurability for the different groups of names:

  • https://docs.astral.sh/ruff/rules/non-pep604-annotation-optional/
  • https://docs.astral.sh/ruff/rules/non-pep604-annotation-union/
  • https://docs.astral.sh/ruff/rules/non-pep585-annotation/

there wouldn't be much value in duplicating this

if you need this behaviour we suggest disabling this rule entirely and configure Ruff accordingly:

[tool.basedpyright]
deprecateTypingAliases = false

[tool.ruff.lint]
select = ["UP006"]

KotlinIsland avatar Nov 12 '25 08:11 KotlinIsland