Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve typing #181

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/fluent.runtime.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ jobs:
python -m pip install wheel
python -m pip install --upgrade pip
python -m pip install .
python -m pip install flake8==6 mypy==1 types-babel types-pytz
python -m pip install flake8==6 mypy==1.1.1 types-babel types-pytz
- name: Install latest fluent.syntax
working-directory: ./fluent.syntax
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/fluent.syntax.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
run: |
python -m pip install --upgrade pip
python -m pip install .
python -m pip install flake8==6 mypy==1
python -m pip install flake8==6 mypy==1.1.1
- name: flake8
working-directory: ./fluent.syntax
run: |
Expand Down
3 changes: 1 addition & 2 deletions fluent.runtime/fluent/runtime/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
from fluent.syntax.ast import Resource

from .bundle import FluentBundle
from .fallback import FluentLocalization, AbstractResourceLoader, FluentResourceLoader

from .fallback import AbstractResourceLoader, FluentLocalization, FluentResourceLoader

__all__ = [
'FluentLocalization',
Expand Down
6 changes: 4 additions & 2 deletions fluent.runtime/fluent/runtime/builtins.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
from typing import Any, Callable, Dict
from typing_extensions import Final

from .types import FluentType, fluent_date, fluent_number

NUMBER = fluent_number
DATETIME = fluent_date
NUMBER: Final = fluent_number
DATETIME: Final = fluent_date


BUILTINS: Dict[str, Callable[[Any], FluentType]] = {
Expand Down
16 changes: 9 additions & 7 deletions fluent.runtime/fluent/runtime/bundle.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
from typing import TYPE_CHECKING, Any, Callable, Dict, List, Sequence, Tuple, Union, cast
from typing_extensions import Literal

import babel
import babel.numbers
import babel.plural
from typing import Any, Callable, Dict, List, TYPE_CHECKING, Tuple, Union, cast
from typing_extensions import Literal

from fluent.syntax import ast as FTL

from .builtins import BUILTINS
Expand All @@ -12,6 +12,8 @@
from .utils import native_to_fluent

if TYPE_CHECKING:
from _typeshed import SupportsItems, SupportsKeysAndGetItem

from .types import FluentNone, FluentType

PluralCategory = Literal['zero', 'one', 'two', 'few', 'many', 'other']
Expand All @@ -33,8 +35,8 @@ class FluentBundle:
"""

def __init__(self,
locales: List[str],
functions: Union[Dict[str, Callable[[Any], 'FluentType']], None] = None,
locales: Sequence[str],
functions: Union['SupportsKeysAndGetItem[str, Callable[[Any], FluentType]]', None] = None,
use_isolating: bool = True):
self.locales = locales
self._functions = {**BUILTINS, **(functions or {})}
Expand Down Expand Up @@ -79,10 +81,10 @@ def _lookup(self, entry_id: str, term: bool = False) -> Message:

def format_pattern(self,
pattern: Pattern,
args: Union[Dict[str, Any], None] = None
args: Union['SupportsItems[str, Any]', None] = None
) -> Tuple[Union[str, 'FluentNone'], List[Exception]]:
if args is not None:
fluent_args = {
fluent_args: Dict[str, Any] = {
argname: native_to_fluent(argvalue)
for argname, argvalue in args.items()
}
Expand Down
27 changes: 15 additions & 12 deletions fluent.runtime/fluent/runtime/fallback.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import codecs
import os
from typing import Any, Callable, Dict, Generator, List, TYPE_CHECKING, Type, Union, cast
from typing import TYPE_CHECKING, Any, Callable, Iterable, Iterator, List, Sequence, Type, Union, cast

from fluent.syntax import FluentParser

from .bundle import FluentBundle

if TYPE_CHECKING:
from _typeshed import SupportsItems, SupportsKeysAndGetItem

from fluent.syntax.ast import Resource

from .types import FluentType


Expand All @@ -21,12 +24,12 @@ class FluentLocalization:

def __init__(
self,
locales: List[str],
resource_ids: List[str],
locales: Sequence[str],
resource_ids: Iterable[str],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just read the other day that a str is also an Iterable{str], we should avoid that type if we really want multiple strings.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a loosely typed language like Python, I'm not sure this can be avoided. There are two options, in my mind: use a union of a bunch of common invariant collections (Union[List[str], Tuple[str], AbstractSet[str]]) or add a runtime check to ensure that a string isn't being passed. I really don't like the first option because you're limiting what the user can pass in (a List[MyStringSubtype] can't be passed in, for instance), which is why I changed this to Sequence[str] and Iterable[str].

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A third option is to convert from str to a list() (which is done in a couple of other places):

    def __init__(
        self,
        locales: Union[str, Sequence[str]],
        resource_ids: Union[str, Iterable[str]],
        resource_loader: 'AbstractResourceLoader',
        use_isolating: bool = False,
        bundle_class: Type[FluentBundle] = FluentBundle,
        functions: Union['SupportsKeysAndGetItem[str, Callable[[Any], FluentType]]', None] = None,
    ):
        self.locales = [locales] if isinstance(locales, str) else list(locales)
        self.resource_ids = [resource_ids] if isinstance(resource_ids, str) else list(resource_ids)

resource_loader: 'AbstractResourceLoader',
use_isolating: bool = False,
bundle_class: Type[FluentBundle] = FluentBundle,
functions: Union[Dict[str, Callable[[Any], 'FluentType']], None] = None,
functions: Union['SupportsKeysAndGetItem[str, Callable[[Any], FluentType]]', None] = None,
):
self.locales = locales
self.resource_ids = resource_ids
Expand All @@ -37,7 +40,7 @@ def __init__(
self._bundle_cache: List[FluentBundle] = []
self._bundle_it = self._iterate_bundles()

def format_value(self, msg_id: str, args: Union[Dict[str, Any], None] = None) -> str:
def format_value(self, msg_id: str, args: Union['SupportsItems[str, Any]', None] = None) -> str:
for bundle in self._bundles():
if not bundle.has_message(msg_id):
continue
Expand All @@ -48,12 +51,12 @@ def format_value(self, msg_id: str, args: Union[Dict[str, Any], None] = None) ->
return cast(str, val) # Never FluentNone when format_pattern called externally
return msg_id

def _create_bundle(self, locales: List[str]) -> FluentBundle:
def _create_bundle(self, locales: Sequence[str]) -> FluentBundle:
return self.bundle_class(
locales, functions=self.functions, use_isolating=self.use_isolating
)

def _bundles(self) -> Generator[FluentBundle, None, None]:
def _bundles(self) -> Iterator[FluentBundle]:
bundle_pointer = 0
while True:
if bundle_pointer == len(self._bundle_cache):
Expand All @@ -64,7 +67,7 @@ def _bundles(self) -> Generator[FluentBundle, None, None]:
yield self._bundle_cache[bundle_pointer]
bundle_pointer += 1

def _iterate_bundles(self) -> Generator[FluentBundle, None, None]:
def _iterate_bundles(self) -> Iterator[FluentBundle]:
for first_loc in range(0, len(self.locales)):
locs = self.locales[first_loc:]
for resources in self.resource_loader.resources(locs[0], self.resource_ids):
Expand All @@ -79,7 +82,7 @@ class AbstractResourceLoader:
Interface to implement for resource loaders.
"""

def resources(self, locale: str, resource_ids: List[str]) -> Generator[List['Resource'], None, None]:
def resources(self, locale: str, resource_ids: Iterable[str]) -> Iterator[List['Resource']]:
"""
Yield lists of FluentResource objects, corresponding to
each of the resource_ids.
Expand All @@ -101,14 +104,14 @@ class FluentResourceLoader(AbstractResourceLoader):
different roots.
"""

def __init__(self, roots: Union[str, List[str]]):
def __init__(self, roots: Union[str, Iterable[str]]):
"""
Create a resource loader. The roots may be a string for a single
location on disk, or a list of strings.
"""
self.roots = [roots] if isinstance(roots, str) else roots
self.roots: Iterable[str] = [roots] if isinstance(roots, str) else roots

def resources(self, locale: str, resource_ids: List[str]) -> Generator[List['Resource'], None, None]:
def resources(self, locale: str, resource_ids: Iterable[str]) -> Iterator[List['Resource']]:
for root in self.roots:
resources: List[Any] = []
for resource_id in resource_ids:
Expand Down
6 changes: 4 additions & 2 deletions fluent.runtime/fluent/runtime/prepare.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from typing import Any, Dict, List
from typing import Any, Dict, Sequence

from fluent.syntax import ast as FTL

from . import resolver


Expand Down Expand Up @@ -29,7 +31,7 @@ def compile_Placeable(self, _: Any, expression: Any, **kwargs: Any) -> Any:
return expression
return resolver.Placeable(expression=expression, **kwargs)

def compile_Pattern(self, _: Any, elements: List[Any], **kwargs: Any) -> Any:
def compile_Pattern(self, _: Any, elements: Sequence[Any], **kwargs: Any) -> Any:
if len(elements) == 1 and isinstance(elements[0], resolver.Placeable):
# Don't isolate isolated placeables
return resolver.NeverIsolatingPlaceable(elements[0].expression)
Expand Down
36 changes: 20 additions & 16 deletions fluent.runtime/fluent/runtime/resolver.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import attr
import contextlib
from typing import Any, Dict, Generator, List, Set, TYPE_CHECKING, Union, cast
from typing import TYPE_CHECKING, Any, Dict, Iterable, Iterator, List, Mapping, Sequence, Set, Union, cast
from typing_extensions import Final, Self

import attr
from fluent.syntax import ast as FTL

from .errors import FluentCyclicReferenceError, FluentFormatError, FluentReferenceError
from .types import FluentType, FluentNone, FluentInt, FluentFloat
from .types import FluentFloat, FluentInt, FluentNone, FluentType
from .utils import reference_to_id, unknown_reference_error_obj

if TYPE_CHECKING:
Expand All @@ -27,7 +29,7 @@


# Prevent expansion of too long placeables, for memory DOS protection
MAX_PART_LENGTH = 2500
MAX_PART_LENGTH: Final = 2500


@attr.s
Expand Down Expand Up @@ -56,7 +58,7 @@ class ResolverEnvironment:
current: CurrentEnvironment = attr.ib(factory=CurrentEnvironment)

@contextlib.contextmanager
def modified(self, **replacements: Any) -> Generator['ResolverEnvironment', None, None]:
def modified(self, **replacements: Any) -> Iterator[Self]:
"""
Context manager that modifies the 'current' attribute of the
environment, restoring the old data at the end.
Expand All @@ -68,7 +70,9 @@ def modified(self, **replacements: Any) -> Generator['ResolverEnvironment', None
yield self
self.current = old_current

def modified_for_term_reference(self, args: Union[Dict[str, Any], None] = None) -> Any:
def modified_for_term_reference(self,
args: Union[Mapping[str, Any], None] = None
) -> 'contextlib._GeneratorContextManager[Self]':
return self.modified(args=args if args is not None else {},
error_for_missing_arg=False)

Expand Down Expand Up @@ -99,7 +103,7 @@ class Message(FTL.Entry, BaseResolver):
def __init__(self,
id: 'Identifier',
value: Union['Pattern', None] = None,
attributes: Union[List['Attribute'], None] = None,
attributes: Union[Iterable['Attribute'], None] = None,
comment: Any = None,
**kwargs: Any):
super().__init__(**kwargs)
Expand All @@ -116,7 +120,7 @@ class Term(FTL.Entry, BaseResolver):
def __init__(self,
id: 'Identifier',
value: 'Pattern',
attributes: Union[List['Attribute'], None] = None,
attributes: Union[Iterable['Attribute'], None] = None,
comment: Any = None,
**kwargs: Any):
super().__init__(**kwargs)
Expand All @@ -129,7 +133,7 @@ class Pattern(FTL.Pattern, BaseResolver):
# Prevent messages with too many sub parts, for CPI DOS protection
MAX_PARTS = 1000

elements: List[Union['TextElement', 'Placeable']] # type: ignore
elements: Sequence[Union['TextElement', 'Placeable']]

def __init__(self, *args: Any, **kwargs: Any):
super().__init__(*args, **kwargs)
Expand Down Expand Up @@ -198,14 +202,14 @@ def __call__(self, env: ResolverEnvironment) -> str:


class NumberLiteral(FTL.NumberLiteral, BaseResolver):
value: Union[FluentFloat, FluentInt] # type: ignore
value: Union[FluentFloat, FluentInt] # type: ignore[assignment]

def __init__(self, value: str, **kwargs: Any):
super().__init__(value, **kwargs)
if '.' in cast(str, self.value):
self.value = FluentFloat(self.value)
if '.' in value:
self.value = FluentFloat(value)
else:
self.value = FluentInt(self.value)
self.value = FluentInt(value)

def __call__(self, env: ResolverEnvironment) -> Union[FluentFloat, FluentInt]:
return self.value
Expand Down Expand Up @@ -285,7 +289,7 @@ class Attribute(FTL.Attribute, BaseResolver):

class SelectExpression(FTL.SelectExpression, BaseResolver):
selector: 'InlineExpression'
variants: List['Variant'] # type: ignore
variants: Sequence['Variant']

def __call__(self, env: ResolverEnvironment) -> Union[str, FluentNone]:
key = self.selector(env)
Expand Down Expand Up @@ -340,8 +344,8 @@ def __call__(self, env: ResolverEnvironment) -> str:


class CallArguments(FTL.CallArguments, BaseResolver):
positional: List[Union['InlineExpression', Placeable]] # type: ignore
named: List['NamedArgument'] # type: ignore
positional: Sequence[Union['InlineExpression', Placeable]]
named: Sequence['NamedArgument']


class FunctionReference(FTL.FunctionReference, BaseResolver):
Expand Down
Loading