union with builtin variant name bug

afloren-palantir commented 1 year ago

Before this PR

the following properties are generated on the union class:

@builtins.property
def float(self) -> Optional[float]:
    return self._float

@builtins.property
def double(self) -> Optional[float]:
    return self._double

however a type error occurs when evaluating the return type of the double method where the float type is interpreted as the class' property, i.e., the following error is thrown on import:

_______________ ERROR collecting test/client/test_import_all.py ________________
test/client/test_import_all.py:1: in <module>
    from generated_integration import *
test/generated_integration/generated_integration_subpackage/__init__.py:2: in <module>
    from .._impl import (
test/generated_integration/_impl.py:1025: in <module>
    class product_UnionWithBuiltinVariantName(ConjureUnionType):
test/generated_integration/_impl.py:1069: in product_UnionWithBuiltinVariantName
    def double(self) -> Optional[float]:
../../../.pyenv/versions/3.8.12/lib/python3.8/typing.py:261: in inner
    return func(*args, **kwds)
../../../.pyenv/versions/3.8.12/lib/python3.8/typing.py:364: in __getitem__
    arg = _type_check(parameters, "Optional[t] requires a single type.")
../../../.pyenv/versions/3.8.12/lib/python3.8/typing.py:149: in _type_check
    raise TypeError(f"{msg} Got {arg!r:.100}.")
E   TypeError: Optional[t] requires a single type. Got <property object at 0x7fa26ff448b0>.

After this PR

float fields/variants are sanitized and the new test case (and all existing test cases) pass

==COMMIT_MSG== ==COMMIT_MSG==

Possible downsides?

Technically this is a break for clients who were accessing fields/variants with the name "float" (notably this behavior was already the case for str, bool, int and other builtin types) but since those clients would have been broken we don't expect this to actually be an issue for any consumers.

changelog-app[bot] commented 1 year ago

Generate changelog in changelog-dir>`changelog/@unreleased`</changelog-dir

What do the change types mean?

- `feature`: A new feature of the service. - `improvement`: An incremental improvement in the functionality or operation of the service. - `fix`: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way. - `break`: Has the potential to break consumers of this service's API, inclusive of both Palantir services and external consumers of the service's API (e.g. customer-written software or integrations). - `deprecation`: Advertises the intention to remove service functionality without any change to the operation of the service itself. - `manualTask`: Requires the possibility of manual intervention (running a script, eyeballing configuration, performing database surgery, ...) at the time of upgrade for it to succeed. - `migration`: A fully automatic upgrade migration task with no engineer input required. _Note: only one type should be chosen._

How are new versions calculated?

- ❗The `break` and `manual task` changelog types will result in a major release! - 🐛 The `fix` changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease. - ✨ All others will result in a minor version release.

Type

- [ ] Feature - [ ] Improvement - [x] Fix - [ ] Break - [ ] Deprecation - [ ] Manual task - [ ] Migration

Description

union with builtin variant name bug

**Check the box to generate changelog(s)** - [x] Generate changelog entry

afloren-palantir commented 1 year ago

since sanitizing would be a client break we are working around with https://github.com/palantir/conjure-python/pull/749

palantir / conjure-python