-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Allow dict(), **, and | to operate on non-dict mappings #13605
Comments
Hi @brandjon - happy to take a stab at a PR to support this if you’re up for it. |
Another wart we'd want to fix here is Note that after taking a quick stab at implementing |
... and another wart: #14540 will add the Python 3.9 style union operator for dicts, which the dict-like views will not support. Thus, I think we need to tackle problem sooner rather than later. |
…y dicts. In particular, this allows immutable view objects returned by native.existing_rule/s() with --incompatible_existing_rules_immutable_view to be used in dict() and dict.update(). Partially addresses #13605 PiperOrigin-RevId: 478527510 Change-Id: Ic796df9d556027c5d797b03ca85aebf1c90165cd
…ry maps, not just dicts In particular, this allows immutable view objects returned by native.existing_rule/s() with --incompatible_existing_rules_immutable_view to be used with | and |=. Partially addresses #13605 PiperOrigin-RevId: 478547382 Change-Id: I141a4bfb97457c5e3ee435d155b0148c3e5ad3ae
…y dicts. In particular, this allows immutable view objects returned by native.existing_rule/s() with --incompatible_existing_rules_immutable_view to be used in dict() and dict.update(). Partially addresses bazelbuild#13605 PiperOrigin-RevId: 478527510 Change-Id: Ic796df9d556027c5d797b03ca85aebf1c90165cd
…ry maps, not just dicts In particular, this allows immutable view objects returned by native.existing_rule/s() with --incompatible_existing_rules_immutable_view to be used with | and |=. Partially addresses bazelbuild#13605 PiperOrigin-RevId: 478547382 Change-Id: I141a4bfb97457c5e3ee435d155b0148c3e5ad3ae
Currently,
dict(v)
allowsv
to be either an iterable of key-value pairs, or another dict object whose entries get included into the new dict. We should additionally allowv
to be a non-dict mapping. In Bazel an example of such a type would beOutputGroupInfo
.See bazelbuild/starlark#198 for the corresponding spec change.
Implementation wise, following the precedent of
StarlarkIterable
vsIterable
, we'd probably want to create a marker interfaceStarlarkMapping
, rather than using the standard JavaMap
interface.StarlarkMapping
would extendStarlarkIterable
andStarlarkIndexable
. Using a marker interface as opposed toMap
gives more control to the implementor; in particular, it lets us say thatOutputGroupInfo
is a mapping without having to implement an extra Java API we don't want it to have. Conversely, it also avoids changing the Starlark semantics of a type whenever its class gains or loses theMap
interface.OutputGroupInfo
would be made to implementStarlarkMapping
instead ofStarlarkIterable
andStarlarkIndexable
(which it would still inherit indirectly). In contrast,TransitiveInfoCollection
would continue to implementStarlarkIndexable
withoutStarlarkIterable
, since it does not support enumeration of providers. Other types, e.g. toolchain contexts, may or may not choose to be classified as a mapping.Implementing
StarlarkMapping
wouldn't obligate you to support, e.g., efficient enumeration of keys via a view object. It's simply the join of iterable and indexable.This issue came up in the discussion of Improving native.existing_rules, where it was pointed out that migrating the return value from a dict object to a dict-like view would require any users who wanted to copy the result to do
dict(view.items())
rather than simplydict(view)
. @tetromino fyi.The text was updated successfully, but these errors were encountered: