Improve converters and type annotations

[injust]

Changes

• Add proper type annotations to converters, instead of Any
• as_timezone: Remove unreachable None handling (fixes Unreachable None handling in as_timezone() converter #1013)
  • It doesn't make sense for None to do the same thing as the "local" default while being less explicit, so I omitted it from the type annotation and dropped the unreachable code
  • The current runtime behaviour is maintained, but a type checker will now warn you about it
• CronTrigger.from_crontab: Swap timezone parameter type annotation ordering to tzinfo | str for consistency
  • This ordering seems more logical than str | tzinfo
• as_enum: Use Enum.__getitem__ instead of Enum.__members__.__getitem__

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

• You've added tests (in tests/) added which would fail without your patch
• You've updated the documentation (in docs/, in case of behavior changes or new
  features)
• You've added a new changelog entry (in docs/versionhistory.rst).

If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.

Updating the changelog

If there are no entries after the last release, use **UNRELEASED** as the version.
If, say, your patch fixes issue #999, the entry should look like this:

* Fix big bad boo-boo in the async scheduler (#999 <https://github.com/agronholm/apscheduler/issues/999>_; PR by @yourgithubaccount)

If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.

[coveralls]

coverage: 92.002% (-0.03%) from 92.033%
when pulling 3334665 on injust:patch-1
into 74a5ba4 on agronholm:master.

[injust]

I'm not sure how to best type list_converter -- going to leave it as is.

[injust]

I diffed the current docs against the RTD build for this PR and the changes look good. For example, DateTrigger()'s run_time argument is now typed as datetime | str instead of Any.

[injust]

But somehow I made sphinx-autodoc-typehints unhappy -- https://apscheduler--1014.org.readthedocs.build/en/1014/api.html#apscheduler.triggers.interval.IntervalTrigger looks like this:

The types aren't in code blocks and don't link to the Python docs anymore. No idea what happened.

[injust]

as_enum() should be typed like this:

def as_enum(enum_class: type[Enum]) -> Callable[[Enum | str], Enum]:
    def converter(value: Enum | str) -> Enum: ...

but it breaks the docs build (both locally and on RTD):

WARNING: Cannot resolve forward reference in type annotations of "apscheduler.Schedule": name 'Enum' is not defined
WARNING: Cannot resolve forward reference in type annotations of "apscheduler.JobResult": name 'Enum' is not defined
WARNING: Cannot resolve forward reference in type annotations of "apscheduler.AsyncScheduler": name 'Enum' is not defined

So I'll undo the type annotations but keep the change from enum_class.__members__[value] to enum_class[value], since that's how enum members should be accessed by name (https://docs.python.org/3/howto/enum.html#programmatic-access-to-enumeration-members-and-their-attributes).

[agronholm]

The type annotations for converters are not appropriate, as converters are run before validators, so the input value could be literally anything.

[injust]

The type annotations for converters are not appropriate, as converters are run before validators, so the input value could be literally anything.

I'm not sure I follow. Type annotations should describe what's intended, so your type checker can catch errors before you get a TypeError from the validator at runtime.

[agronholm]

The type annotations for converters are not appropriate, as converters are run before validators, so the input value could be literally anything.

I'm not sure I follow. Type annotations should describe what's intended, so your type checker can catch errors before you get a TypeError from the validator at runtime.

Sure, but does the type checker know that it should look at the attrs converter to determine what types are allowed as input?

[injust]

Sure, but does the type checker know that it should look at the attrs converter to determine what types are allowed as input?

Absolutely. I guess the attrs docs aren't the clearest about this, but a field's type annotation is overridden by that of its converter. https://www.attrs.org/en/stable/init.html#converters says:

If a converter’s first argument has a type annotation, that type will appear in the signature for __init__. A converter will override an explicit type annotation or type argument.

Which means that currently, converted fields are typed as Any (to a type checker as well as in the docs).

After this PR:

[agronholm]

Since you've verified that these changes improve the type checking and documentation, I'm merging this. Thanks!