Skip to content

Commit

Permalink
Misc cleanup, see commit body (#227)
Browse files Browse the repository at this point in the history
- Enable oneof_enum test case that passes now (removed the xfail)
- Switch from toml to tomlkit as a dev dep for better toml support
- upgrade poethepoet to latest stable release
- use full table format for poe tasks to avoid long lines in pyproject.toml
- remove redundant _WrappedMessage class
- fix various Mypy warnings
- reformat some comments for consistent line length
  • Loading branch information
nat-n authored Apr 6, 2021
1 parent 5b639c8 commit 95339bf
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 120 deletions.
134 changes: 67 additions & 67 deletions poetry.lock

Large diffs are not rendered by default.

65 changes: 49 additions & 16 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ jinja2 = { version = "^2.11.2", optional = true }
python-dateutil = "^2.8"

[tool.poetry.dev-dependencies]
asv = "^0.4.2"
black = "^20.8b1"
bpython = "^0.19"
grpcio-tools = "^1.30.0"
Expand All @@ -31,11 +32,10 @@ pytest = "^5.4.2"
pytest-asyncio = "^0.12.0"
pytest-cov = "^2.9.0"
pytest-mock = "^3.1.1"
toml = "^0.10.1"
tox = "^3.15.1"
sphinx = "3.1.2"
sphinx-rtd-theme = "0.5.0"
asv = "^0.4.2"
tomlkit = "^0.7.0"
tox = "^3.15.1"


[tool.poetry.scripts]
Expand All @@ -44,29 +44,62 @@ protoc-gen-python_betterproto = "betterproto.plugin:main"
[tool.poetry.extras]
compiler = ["black", "jinja2"]

[tool.poe.tasks]

# Dev workflow tasks
generate = { script = "tests.generate:main", help = "Generate test cases (do this once before running test)" }
test = { cmd = "pytest --cov src", help = "Run tests" }
types = { cmd = "mypy src --ignore-missing-imports", help = "Check types with mypy" }
format = { cmd = "black . --exclude tests/output_", help = "Apply black formatting to source code" }
docs = { cmd = "sphinx-build docs docs/build", help = "Build the sphinx docs"}
bench = { shell = "asv run master^! && asv run HEAD^! && asv compare master HEAD", help = "Benchmark current commit vs. master branch"}
clean = { cmd = "rm -rf .asv .coverage .mypy_cache .pytest_cache dist betterproto.egg-info **/__pycache__ tests/output_*", help = "Clean out generated files from the workspace" }

generate_lib.cmd = """

[tool.poe.tasks.generate]
script = "tests.generate:main"
help = "Generate test cases (do this once before running test)"

[tool.poe.tasks.test]
cmd = "pytest"
help = "Run tests"

[tool.poe.tasks.types]
cmd = "mypy src --ignore-missing-imports"
help = "Check types with mypy"

[tool.poe.tasks.format]
cmd = "black . --exclude tests/output_"
help = "Apply black formatting to source code"

[tool.poe.tasks.docs]
cmd = "sphinx-build docs docs/build"
help = "Build the sphinx docs"

[tool.poe.tasks.bench]
shell = "asv run master^! && asv run HEAD^! && asv compare master HEAD"
help = "Benchmark current commit vs. master branch"

[tool.poe.tasks.clean]
cmd = """
rm -rf .asv .coverage .mypy_cache .pytest_cache
dist betterproto.egg-info **/__pycache__
testsoutput_*
"""
help = "Clean out generated files from the workspace"

[tool.poe.tasks.generate_lib]
cmd = """
protoc
--plugin=protoc-gen-custom=src/betterproto/plugin/main.py
--custom_opt=INCLUDE_GOOGLE
--custom_out=src/betterproto/lib
-I /usr/local/include/
/usr/local/include/google/protobuf/**/*.proto
"""
generate_lib.help = "Regenerate the types in betterproto.lib.google"
help = "Regenerate the types in betterproto.lib.google"

# CI tasks
full-test = { shell = "poe generate && tox", help = "Run tests with multiple pythons" }
check-style = { cmd = "black . --check --diff --exclude tests/output_", help = "Check if code style is correct"}

[tool.poe.tasks.full-test]
shell = "poe generate && tox"
help = "Run tests with multiple pythons"

[tool.poe.tasks.check-style]
cmd = "black . --check --diff --exclude tests/output_"
help = "Check if code style is correct"


[tool.black]
target-version = ['py36']
Expand Down
32 changes: 8 additions & 24 deletions src/betterproto/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
Callable,
Dict,
Generator,
Iterable,
List,
Optional,
Set,
Expand Down Expand Up @@ -272,7 +273,7 @@ def from_string(cls, name: str) -> "Enum":
The member was not found in the Enum.
"""
try:
return cls._member_map_[name]
return cls._member_map_[name] # type: ignore
except KeyError as e:
raise ValueError(f"Unknown value {name} for enum {cls.__name__}") from e

Expand Down Expand Up @@ -522,13 +523,13 @@ def __init__(self, cls: Type["Message"]):

@staticmethod
def _get_default_gen(
cls: Type["Message"], fields: List[dataclasses.Field]
cls: Type["Message"], fields: Iterable[dataclasses.Field]
) -> Dict[str, Callable[[], Any]]:
return {field.name: cls._get_field_default_gen(field) for field in fields}

@staticmethod
def _get_cls_by_field(
cls: Type["Message"], fields: List[dataclasses.Field]
cls: Type["Message"], fields: Iterable[dataclasses.Field]
) -> Dict[str, Type]:
field_cls = {}

Expand Down Expand Up @@ -687,7 +688,7 @@ def _betterproto(self) -> ProtoClassMetadata:
meta = getattr(self.__class__, "_betterproto_meta", None)
if not meta:
meta = ProtoClassMetadata(self.__class__)
self.__class__._betterproto_meta = meta
self.__class__._betterproto_meta = meta # type: ignore
return meta

def __bytes__(self) -> bytes:
Expand Down Expand Up @@ -763,7 +764,7 @@ def __bytes__(self) -> bytes:
meta.number,
meta.proto_type,
value,
serialize_empty=serialize_empty or selected_in_group,
serialize_empty=serialize_empty or bool(selected_in_group),
wraps=meta.wraps or "",
)

Expand Down Expand Up @@ -1067,7 +1068,7 @@ def to_dict(
output[cased_name] = b64encode(value).decode("utf8")
elif meta.proto_type == TYPE_ENUM:
if field_is_repeated:
enum_class: Type[Enum] = field_types[field_name].__args__[0]
enum_class = field_types[field_name].__args__[0]
if isinstance(value, typing.Iterable) and not isinstance(
value, str
):
Expand All @@ -1076,7 +1077,7 @@ def to_dict(
# transparently upgrade single value to repeated
output[cased_name] = [enum_class(value).name]
else:
enum_class: Type[Enum] = field_types[field_name] # noqa
enum_class = field_types[field_name] # noqa
output[cased_name] = enum_class(value).name
elif meta.proto_type in (TYPE_FLOAT, TYPE_DOUBLE):
if field_is_repeated:
Expand Down Expand Up @@ -1293,23 +1294,6 @@ def timestamp_to_json(dt: datetime) -> str:
return f"{result}.{nanos:09d}"


class _WrappedMessage(Message):
"""
Google protobuf wrapper types base class. JSON representation is just the
value itself.
"""

value: Any

def to_dict(self, casing: Casing = Casing.CAMEL) -> Any:
return self.value

def from_dict(self: T, value: Any) -> T:
if value is not None:
self.value = value
return self


def _get_wrapper(proto_type: str) -> Type:
"""Get the wrapper message class for a wrapped type."""

Expand Down
16 changes: 9 additions & 7 deletions src/betterproto/compile/importing.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ def reference_descendent(
current_package: List[str], imports: Set[str], py_package: List[str], py_type: str
) -> str:
"""
Returns a reference to a python type in a package that is a descendent of the current package,
and adds the required import that is aliased to avoid name conflicts.
Returns a reference to a python type in a package that is a descendent of the
current package, and adds the required import that is aliased to avoid name
conflicts.
"""
importing_descendent = py_package[len(current_package) :]
string_from = ".".join(importing_descendent[:-1])
Expand All @@ -119,8 +120,9 @@ def reference_ancestor(
current_package: List[str], imports: Set[str], py_package: List[str], py_type: str
) -> str:
"""
Returns a reference to a python type in a package which is an ancestor to the current package,
and adds the required import that is aliased (if possible) to avoid name conflicts.
Returns a reference to a python type in a package which is an ancestor to the
current package, and adds the required import that is aliased (if possible) to avoid
name conflicts.
Adds trailing __ to avoid name mangling (python.org/dev/peps/pep-0008/#id34).
"""
Expand All @@ -141,10 +143,10 @@ def reference_cousin(
current_package: List[str], imports: Set[str], py_package: List[str], py_type: str
) -> str:
"""
Returns a reference to a python type in a package that is not descendent, ancestor or sibling,
and adds the required import that is aliased to avoid name conflicts.
Returns a reference to a python type in a package that is not descendent, ancestor
or sibling, and adds the required import that is aliased to avoid name conflicts.
"""
shared_ancestry = os.path.commonprefix([current_package, py_package])
shared_ancestry = os.path.commonprefix([current_package, py_package]) # type: ignore
distance_up = len(current_package) - len(shared_ancestry)
string_from = f".{'.' * distance_up}" + ".".join(
py_package[len(shared_ancestry) : -1]
Expand Down
2 changes: 1 addition & 1 deletion src/betterproto/grpc/util/async_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class AsyncChannel(AsyncIterable[T]):
"""

def __init__(self, *, buffer_limit: int = 0, close: bool = False):
self._queue: asyncio.Queue[Union[T, object]] = asyncio.Queue(buffer_limit)
self._queue: asyncio.Queue[T] = asyncio.Queue(buffer_limit)
self._closed = False
self._waiting_receivers: int = 0
# Track whether flush has been invoked so it can only happen once
Expand Down
2 changes: 2 additions & 0 deletions src/betterproto/plugin/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ class ProtoContentBase:
comment_indent: int = 4
parent: Union["betterproto.Message", "OutputTemplate"]

__dataclass_fields__: Dict[str, object]

def __post_init__(self) -> None:
"""Checks that no fake default fields were left as placeholders."""
for field_name, field_val in self.__dataclass_fields__.items():
Expand Down
4 changes: 2 additions & 2 deletions src/betterproto/plugin/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import itertools
import pathlib
import sys
from typing import Iterator, List, Tuple, TYPE_CHECKING, Union
from typing import Iterator, List, Set, Tuple, TYPE_CHECKING, Union
from .compiler import outputfile_compiler
from .models import (
EnumDefinitionCompiler,
Expand All @@ -38,7 +38,7 @@ def traverse(
) -> "itertools.chain[Tuple[Union[str, EnumDescriptorProto], List[int]]]":
# Todo: Keep information about nested hierarchy
def _traverse(
path: List[int], items: List["Descriptor"], prefix=""
path: List[int], items: List["EnumDescriptorProto"], prefix=""
) -> Iterator[Tuple[Union[str, EnumDescriptorProto], List[int]]]:
for i, item in enumerate(items):
# Adjust the name since we flatten the hierarchy.
Expand Down
1 change: 0 additions & 1 deletion tests/inputs/config.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Test cases that are expected to fail, e.g. unimplemented features or bug-fixes.
# Remove from list when fixed.
xfail = {
"oneof_enum", # 63
"namespace_keywords", # 70
"namespace_builtin_types", # 53
"googletypes_struct", # 9
Expand Down
4 changes: 2 additions & 2 deletions tests/test_version.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
from betterproto import __version__
from pathlib import Path
import toml
import tomlkit

PROJECT_TOML = Path(__file__).joinpath("..", "..", "pyproject.toml").resolve()


def test_version():
with PROJECT_TOML.open() as toml_file:
project_config = toml.load(toml_file)
project_config = tomlkit.loads(toml_file.read())
assert (
__version__ == project_config["tool"]["poetry"]["version"]
), "Project version should match in package and package config"

0 comments on commit 95339bf

Please sign in to comment.