Skip to content

Conversation

@afloren-palantir
Copy link
Contributor

@afloren-palantir afloren-palantir commented Aug 8, 2023

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
Copy link

changelog-app bot commented Aug 8, 2023

Generate changelog in changelog/@unreleased

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
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

union with builtin variant name bug

Check the box to generate changelog(s)

  • Generate changelog entry

@afloren-palantir
Copy link
Contributor Author

since sanitizing would be a client break we are working around with #749

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants