Skip to content

Commit

Permalink
Parameter default type-checking reversion.
Browse files Browse the repository at this point in the history
This commit temporarily reverts the prior commit chain type-checking the
default values of optional parameters accepted by `@beartype`-decorated
callables, resolving issue #354 kindly submitted by the
Minos-labyrinth-projected-onto-a-tesseract @rbnhd (Vikram Kushwaha)
*and* re-opening feature request #154 kindly submitted by @dosisod
(Logan Hunt) a literal lifetime ago when @leycec still had hair.
Although highly desirable, this functionality is also considerably more
nuanced than @leycec previously assumed -- resulting in widespread
breakage throughout the downstream ecosystem of @beartype consumers.
That's demonstrably awful. Since properly improving this functionality
to be robust against that breakage is non-trivial, the proper solution
is simply to temporarily disable that functionality. Thus, we cry.
(*Humorous hemorrhage or us!*)
  • Loading branch information
leycec committed Apr 3, 2024
1 parent cdccff8 commit 7b4976a
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 76 deletions.
2 changes: 1 addition & 1 deletion beartype/_check/code/codemake.py
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@ def _enqueue_hint_child(pith_child_expr: str) -> str:
# or efficient means of retrieving the only item
# of a 1-set. Indeed, the most efficient means
# of doing so is to iterate over that set and
# break:
# immediately halt iteration:
# for first_item in muh_set: break
#
# While we *COULD* technically leverage that
Expand Down
112 changes: 65 additions & 47 deletions beartype/_check/code/codescope.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@
from beartype.roar import BeartypeDecorHintNonpepException
from beartype.typing import (
Dict,
List,
Optional,
Tuple,
)
from beartype._cave._cavemap import NoneTypeOr
from beartype._check.forward.reference.fwdrefmake import (
make_forwardref_indexable_subtype)
from beartype._check.forward.reference.fwdrefmeta import BeartypeForwardRefMeta
from beartype._check.code.snip.codesnipstr import (
CODE_HINT_REF_TYPE_BASENAME_PLACEHOLDER_PREFIX,
CODE_HINT_REF_TYPE_BASENAME_PLACEHOLDER_SUFFIX,
Expand Down Expand Up @@ -298,48 +300,9 @@ def add_func_scope_types(
refer to the passed set or tuple of classes and whose value is that tuple)
to the passed scope *and* return that machine-readable name.
This function additionally caches this tuple with the beartypistry
singleton to reduce space consumption for tuples duplicated across the
active Python interpreter.
Design
------
Unlike types, tuples are commonly dynamically constructed on-the-fly by
various tuple factories (e.g., :attr:`beartype.cave.NoneTypeOr`,
:attr:`typing.Optional`) and hence have no reliable fully-qualified names.
Instead, this function caches this tuple into the beartypistry under a
string synthesized as the unique concatenation of:
* The magic substring :data:`TYPISTRY_HINT_NAME_TUPLE_PREFIX`. Since
fully-qualified classnames uniquely identifying types as beartypistry keys
are guaranteed to *never* contain this substring, this substring prevents
collisions between tuple and type names.
* This tuple's hash. Note that this tuple's object ID is intentionally *not*
embedded in this string. Two tuples with the same items are typically
different objects and thus have different object IDs, despite producing
identical hashes: e.g.,
>>> ('Das', 'Kapitel',) is ('Das', 'Kapitel',)
False
>>> id(('Das', 'Kapitel',)) == id(('Das', 'Kapitel',))
False
>>> hash(('Das', 'Kapitel',)) == hash(('Das', 'Kapitel',))
True
The exception is the empty tuple, which is a singleton and thus *always* has
the same object ID and hash: e.g.,
>>> () is ()
True
>>> id(()) == id(())
True
>>> hash(()) == hash(())
True
Identifying tuples by their hashes enables the beartypistry singleton to
transparently cache duplicate class tuples with distinct object IDs as the
same underlying object, reducing space consumption. While hashing tuples
does impact time performance, the gain in space is worth the cost.
This function additionally caches this tuple with the
:data:`._tuple_union_to_tuple_union` dictionary to reduce space consumption
for tuples duplicated across the active Python interpreter.
Parameters
----------
Expand Down Expand Up @@ -393,8 +356,9 @@ def add_func_scope_types(
generate name collisions. This exception is thus intentionally raised
as a private rather than public exception.
'''
assert is_unique.__class__ is bool, f'{repr(is_unique)} not bool.'
assert isinstance(is_unique, bool), f'{repr(is_unique)} not bool.'

# ....................{ VALIDATE }....................
# If this container is neither a set nor tuple, raise an exception.
if not isinstance(types, TYPES_SET_OR_TUPLE):
raise BeartypeDecorHintNonpepException(
Expand All @@ -418,10 +382,62 @@ def add_func_scope_types(
)
# Else, this container either contains two or more types.

# If this container is a frozenset, coerce this frozenset into a tuple.
# ....................{ FORWARDREF }....................
#FIXME: Unit test this up, please.
# True only if this container contains one or more beartype-specific forward
# reference proxies. Although these proxies are technically isinstanceable
# classes, attempting to pass these proxies as the second parameter to the
# isinstance() builtin also raises exceptions when the underlying
# user-defined classes proxied by these proxies have yet to be declared.
# Since these proxies are thus *MUCH* more fragile than standard classes, we
# reduce the likelihood of exceptions by deprioritizing these proxies in
# this container (i.e., moving these proxies to the end of this container).
is_types_ref = False

# For each type in this container...
for cls in types:
# If this type is a beartype-specific forward reference proxy...
if cls.__class__ is BeartypeForwardRefMeta:
# Note that this container contains at least one such proxy.
is_types_ref = False

# Halt iteration.
break

# If this container contains at least one such proxy...
if is_types_ref:
# List of all such proxies in this container.
#
# Note that we intentionally avoid instantiating this pair of lists
# above in the common case that this container contains no such proxies.
types_ref: List[type] = []

# List of all other types in this container (i.e., normal types that are
# *NOT* beartype-specific forward references).
types_nonref: List[type] = []

# For each type in this container...
for cls in types:
# If this type is such a proxy, append this proxy to the list of all
# such proxies.
if cls.__class__ is BeartypeForwardRefMeta:
types_ref.append(cls)
# Else, this type is *NOT* such a proxy. In this case, append this
# proxy to the list of all non-proxy types.
else:
types_nonref.append(cls)

# Reconstitute this container such that all non-proxy types appear
# *BEFORE* all proxy types.
types = tuple(types_nonref + types_ref)
# Else, this container contains *NO* such proxies. In this case, preserve
# the ordering of items in this container as is.

# ....................{ DUPLICATES }....................
# If this container is a set, coerce this frozenset into a tuple.
if isinstance(types, Set):
types = tuple(types)
# Else, this container is *NOT* a frozenset. By elimination, this container
# Else, this container is *NOT* a set. By elimination, this container
# should now be a tuple.
#
# In either case, this container should now be a tuple.
Expand All @@ -438,11 +454,12 @@ def add_func_scope_types(
# * Back into a duplicate-free tuple.
if isinstance(types, tuple) and not is_unique:
types = tuple(set(types))
# In either case, this container is now guaranteed to be a tuple
# containing only duplicate-free classes.
# In either case, this container is now guaranteed to be a tuple containing
# only duplicate-free classes.
assert isinstance(types, tuple), (
f'{exception_prefix}{repr(types)} not tuple.')

# ....................{ CACHE }....................
# If this tuple has *NOT* already been cached, do so.
if types not in _tuple_union_to_tuple_union:
_tuple_union_to_tuple_union[types] = types
Expand All @@ -451,6 +468,7 @@ def add_func_scope_types(
else:
types = _tuple_union_to_tuple_union[types]

# ....................{ RETURN }....................
# Return the name of a new parameter passing this tuple.
return add_func_scope_attr(
attr=types, func_scope=func_scope, exception_prefix=exception_prefix)
Expand Down
26 changes: 18 additions & 8 deletions beartype/_decor/wrap/_wrapargs.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@

# ....................{ IMPORTS }....................
from beartype.roar import (
BeartypeCallHintForwardRefException,
BeartypeDecorHintParamDefaultForwardRefWarning,
BeartypeDecorHintParamDefaultViolation,
BeartypeDecorHintPepException,
BeartypeDecorParamNameException,
)
from beartype.roar._roarexc import _BeartypeHintForwardRefExceptionMixin
from beartype._check.checkcall import BeartypeCall
from beartype._check.checkmake import make_code_raiser_func_pith_check
from beartype._check.convert.convsanify import sanify_hint_root_func
Expand Down Expand Up @@ -200,12 +200,22 @@ def code_check_args(bear_call: BeartypeCall) -> str:
continue
# Else, this hint is unignorable.

# If this parameter is optional *AND* the default value of this
# optional parameter violates this hint, raise an exception.
_die_if_arg_default_unbearable(
bear_call=bear_call, arg_default=arg_default, hint=hint)
# Else, this parameter is either optional *OR* the default value
# of this optional parameter satisfies this hint.
#FIXME: Fundamentally unsafe and thus temporarily disabled *FOR
#THE MOMENT.* The issue is that our current implementation of
#the is_bearable() tester internally called by this function
#refuses to resolve relative forward references -- which is
#obviously awful. Ideally, that tester *ABSOLUTELY* should
#resolve relative forward references. Until it does, however,
#this is verboten dark magic that is unsafe in the general case.
#FIXME: Once this has been repaired, please reenable:
#* The "test_decor_arg_kind_flex_optional" unit test.

# # If this parameter is optional *AND* the default value of this
# # optional parameter violates this hint, raise an exception.
# _die_if_arg_default_unbearable(
# bear_call=bear_call, arg_default=arg_default, hint=hint)
# # Else, this parameter is either optional *OR* the default value
# # of this optional parameter satisfies this hint.

# If this parameter either may *OR* must be passed positionally,
# record this fact.
Expand Down Expand Up @@ -436,7 +446,7 @@ def _die_if_arg_default_unbearable(
# necessarily constitute a fatal error from the end user perspective, this
# does constitute a non-fatal issue worth informing the end user of. In this
# case, we coerce this exception into a warning.
except BeartypeCallHintForwardRefException as exception:
except _BeartypeHintForwardRefExceptionMixin as exception:
# Forward hint exception message raised above. To readably embed this
# message in the longer warning message emitted below, the first
# character of this message is lowercased as well.
Expand Down
23 changes: 21 additions & 2 deletions beartype/roar/_roarexc.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,23 @@
#!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
from abc import ABCMeta as _ABCMeta

# ....................{ PRIVATE ~ mixins }....................
class _BeartypeHintForwardRefExceptionMixin(Exception, metaclass=_ABCMeta):
'''
Mixin of all **beartype forward reference exceptions** (i.e., exceptions
concerning parent type hints containing one or more forward references to
child type hints that have yet to be declared).
This mixin enables internal logic throughout the :mod:`beartype` codebase to
conveniently, efficiently, and transparently handle *all* forward reference
exceptions -- including:
* :exc:`.BeartypeCallHintForwardRefException`.
* :exc:`.BeartypeDecorHintForwardRefException`.
'''

pass

# ....................{ SUPERCLASS }....................
class BeartypeException(Exception, metaclass=_ABCMeta):
'''
Expand Down Expand Up @@ -129,7 +146,8 @@ class BeartypeDecorHintException(BeartypeDecorException):
pass


class BeartypeDecorHintForwardRefException(BeartypeDecorHintException):
class BeartypeDecorHintForwardRefException(
BeartypeDecorHintException, _BeartypeHintForwardRefExceptionMixin):
'''
**Beartype decorator forward reference type hint exception.**
Expand Down Expand Up @@ -488,7 +506,8 @@ class BeartypeCallHintException(BeartypeCallException):
pass


class BeartypeCallHintForwardRefException(BeartypeCallHintException):
class BeartypeCallHintForwardRefException(
BeartypeCallHintException, _BeartypeHintForwardRefExceptionMixin):
'''
**Beartype type-checking forward reference exception.**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@ def test_pep484_ref_data() -> None:
from beartype_test.a00_unit.data.hint.data_hintref import (
TheDarkestEveningOfTheYear,
WithSluggishSurge,
a_little_shallop,
but_i_have_promises,
its_fields_of_snow,
of_easy_wind,
stopping_by_woods_on,
the_woods_are_lovely,
its_fields_of_snow,
the_dry_leaf,
the_woods_are_lovely,
winding_among_the_springs,
# between_the_woods_and_frozen_lake,
)
Expand All @@ -56,6 +57,7 @@ def test_pep484_ref_data() -> None:

# ..................{ PASS }..................
# Assert these forward-referencing callables return the expected values.
assert a_little_shallop(WITH_BURNING_SMOKE) is WITH_BURNING_SMOKE
assert but_i_have_promises(MILES_TO_GO) is MILES_TO_GO
assert of_easy_wind(WOODS) is WOODS
assert stopping_by_woods_on(LAKE) is LAKE
Expand Down
2 changes: 2 additions & 0 deletions beartype_test/a00_unit/a70_decor/a40_code/test_codeargkind.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
# WARNING: To raise human-readable test errors, avoid importing from
# package-specific submodules at module scope.
#!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
from beartype_test._util.mark.pytskip import skip

# ....................{ TESTS ~ name }....................
def test_decor_arg_name_fail() -> None:
Expand Down Expand Up @@ -76,6 +77,7 @@ def special_plan(
)


@skip('Currently broken due to known issues in decoration-time type-checking.')
def test_decor_arg_kind_flex_optional() -> None:
'''
Test the :func:`beartype.beartype` decorator on a callable passed an
Expand Down

0 comments on commit 7b4976a

Please sign in to comment.