Any chance of getting https://github.com/flyteorg/flytekit/pull/2299 merged? We dont see any maintainers being supportive and the only review so far was done by @cool-lifeguard-49380 who I think would generally be in favor of getting this done eventually (thanks, btw). Before further working on the comments I would love to get some signal on the chances of getting this eventually merged.

I’ve been in discussions with @prehistoric-kangaroo-27449 about splitting some of the elaborate logic in the

to_literal

and

to_python_value

methods into smaller reusable functions to make it easier to understand what is happening.

Yes, I am just wondering if resolving this would actually lead to the PR getting merged

@high-accountant-32689 you have reviewed the PR before, the logic you had concerns about back then (overwriting the enum type transformer and having custom logic to detect the right type transformers) has been removed. Could you please take another brief look to check whether there are other blockers from your side apart from this? 🙇

I'll prioritize reviewing this today.

Sorry to bother you @high-accountant-32689, I’m sure you have a lot todo atm 🙈 @prehistoric-kangaroo-27449 would have some time to work on this this week and is looking for feedback whether there are any other blockers apart from what I remarked regarding breaking the logic in

to_literal

and

to_python_value

into smaller and easier to understand chunks.

Great. My feedback is around that too. Let me actually leave comments in the PR.

Thank you 🙇

Thank you both! I'll have a look tomorrow and address the requested changes, hopefully we are then on the same page and can merge the plugin soon 🙂

Thanks, @prehistoric-kangaroo-27449, I left a few comments on the PR. This is looking good, let's get on the same page on some of the open questions and get this change merged.

Alright, I've addressed the issues raised, namely: • refactor transformer mode to be set globally more elegantly, while also providing a local option via context manager • modularize the dict transformer class into smaller chunks, which are individually unit tested • smaller styling changes

@prehistoric-kangaroo-27449, thank you for your change, I have only one comment and then we can merge this, ok?

Done

quick update: an unrelated issue to your PR is impacting the CI checks. I'll ping here when that is solved and tests run.