Support mashumaro DataClassORJSONMixin

[quinten-flwls]

Why are the changes needed?

The mashumaro DataClassORJSONMixin class works like the mashumaro DataClassJSONMixin class. Additionally, it allows for serializing dataclasses with a wider variety of attributes types, including:

• numpy
• datetime
• date
• time
• uuid.UUID

Currently, flytekit does not yet support the DataClassJSONMixin class and throws an error when classes derived from DataClassORJSONMixin are serialized.

What changes were proposed in this pull request?

This PR introduces support for classes derived from the mashumaro DataClassORJSONMixins class.

How was this patch tested?

Added DataClassORJSONMixin instances in test_type_engine.py

Setup process

Screenshots

Check all the applicable boxes

• I updated the documentation accordingly.
• All new and existing tests passed.
• All commits are signed-off.

Related PRs

Docs link

[welcome]

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

• Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
• Sign off your commits (Reference: DCO Guide).

[thomasjpfan]

Thank you for the PR!

For the CI to run, we need to add orjson as a developer dependency in https://github.com/flyteorg/flytekit/blob/master/dev-requirements.in

As for testing, can you add a test that checks the expected behavior for your NumPy use case?

[thomasjpfan]

I prefer not to add orjson as a hard dependency for flytekit. Can you write the PR in way where orjson is a soft dependency?

[quinten-flwls]

Great idea, orjson is a soft dependency now

[quinten-flwls]

Thank you for the PR!

For the CI to run, we need to add orjson as a developer dependency in https://github.com/flyteorg/flytekit/blob/master/dev-requirements.in

As for testing, can you add a test that checks the expected behavior for your NumPy use case?

Hi Thomas, thank you for your feedback! I will incorporate it. As for dev dependendies, we need both orjson and mashumaro>=3.11. What is the best way for me to specify a minimal version in the dev dependencies?

[Fatal1ty]

👋 Hi, author of mashumaro here.

Mashumaro 3.11 introduced the DataClassORJSONMixin class

As for dev dependendies, we need both orjson and mashumaro>=3.11. What is the best way for me to specify a minimal version in the dev dependencies?

To avoid all possible unwanted changes made on false conclusions, I’d like to make a correction. DataClassORJSONMixin was added earlier in 3.1 (see release notes). I hope it’s not that important in case of this pull request and I apologize for bothering you.

[quinten-flwls]

👋 Hi, author of mashumaro here.

Mashumaro 3.11 introduced the DataClassORJSONMixin class

As for dev dependendies, we need both orjson and mashumaro>=3.11. What is the best way for me to specify a minimal version in the dev dependencies?

To avoid all possible unwanted changes made on false conclusions, I’d like to make a correction. DataClassORJSONMixin was added earlier in 3.1 (see release notes). I hope it’s not that important in case of this pull request and I apologize for bothering you.

Hi @Fatal1ty! Thank you for reaching out and reminding me that DataClassORJSONMixin does not require mashumaro 3.11. That is encredibly useful information indeed. I updated the PR description.

The reason why I had mashumaro 3.11 in mind is because we internally use json_encode and json_decode to process DataClassORJSONMixin instances. But that is completely unrelated to this PR.

[thomasjpfan]

Thanks for the update!

wider variety of attributes, including numpy arrays.

Does your NumPy array use case work out of the box after this PR? If so, can you add a test where a NumPy array is an attribute?

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (7996c2e) 82.58% compared to head (95ede28) 85.15%.
Report is 10 commits behind head on master.

Files	Patch %	Lines
flytekit/core/type_engine.py	63.63%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2080      +/-   ##
==========================================
+ Coverage   82.58%   85.15%   +2.56%     
==========================================
  Files         233      284      +51     
  Lines       19541    22197    +2656     
  Branches     3512     3520       +8     
==========================================
+ Hits        16138    18901    +2763     
+ Misses       2824     2686     -138     
- Partials      579      610      +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[quinten-flwls]

Thanks for the update!

wider variety of attributes, including numpy arrays.

Does your NumPy array use case work out of the box after this PR? If so, can you add a test where a NumPy array is an attribute?

Thank you for reviewing! Unfortunately, Numpy array attributes do not work out of the box. However, other attribute types like datetime do. I added a datetime attribute as an example in the tests. I hope that suffices to demonstrate the use case. Let me know if it does not!

[thomasjpfan]

Minor nits, but otherwise LGTM

[welcome]

Congrats on merging your first pull request! 🎉

[quinten-flwls]

Thank you for the feedback and the help!