zero icon indicating copy to clipboard operation
zero copied to clipboard

Add GenericEncoder and Pydantic support

Open martincpt opened this issue 8 months ago • 7 comments

This PR introduces built-in Pydantic support via a new GenericEncoder that is compatible with both Pydantic V1 and V2.

Key changes:

  • GenericEncoder is now the default encoder.
  • Pydantic has been added as an optional dependency and can be installed with:
pip install zeroapi[pydantic]
  • Improved type annotations across the encoders for better clarity and type checking.

martincpt avatar Apr 27 '25 14:04 martincpt

Additional updates:

  • Added missing test using a Pydantic model.
  • Increased coverage for GenericEncoder.
  • Extended the codegen tool to support Pydantic BaseModel, and updated the codegen tests accordingly.

Other notes:

  • Verified the lint command in the Makefile.
  • Could not verify the Makefile's docker-test commands as the necessary Dockerfiles are missing from the repository.
  • When running the Makefile's format command, it reformats files I didn’t modify — mostly adjusting ellipses (...). This might be due to a newer version of black. Would you like me to include these formatting changes in the PR as well?

martincpt avatar May 01 '25 14:05 martincpt

Nice and neat work! :raised_hands:

Ananto30 avatar May 04 '25 07:05 Ananto30

Additional updates:

  • Extended the Encoder protocol with an is_allowed_type method
  • Both the original MsgspecEncoder and the new GenericEncoder implement the type_util.is_allowed_type function
  • type_util.verify_function_input_type and type_util.verify_function_return_type now accept the encoder instance to validate against
  • Tests have been updated accordingly

@Ananto30 Could you please review when you have time?

Also, I'm not sure why Codacy Static Code Analysis is flagging tests/requirements.txt. I only added pydantic, so it might be a false positive — could you take a quick look? Not sure how to ignore that one properly.

Thanks! 🙏

martincpt avatar May 13 '25 10:05 martincpt

Codecov Report

Attention: Patch coverage is 93.75000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 97.46%. Comparing base (d19eaac) to head (e095b8e).

Files with missing lines Patch % Lines
zero/encoder/generic.py 84.21% 3 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #63      +/-   ##
==========================================
- Coverage   97.72%   97.46%   -0.27%     
==========================================
  Files          24       25       +1     
  Lines         836      868      +32     
==========================================
+ Hits          817      846      +29     
- Misses         19       22       +3     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar May 14 '25 06:05 codecov[bot]

Hi @martincpt , very few lines are missing in code coverage. I can merge without them if you want. Then will update and make a release.

Ananto30 avatar Jun 02 '25 12:06 Ananto30

Hi @Ananto30, I covered codegen.py with my latest commit. I've also found solution to cover Pydantic V1 import but it does not tests the code with V1 actually.

I'm currently a little busy. Please give me some time and I'll get back at you.

martincpt avatar Jun 05 '25 08:06 martincpt

Hi @Ananto30, sorry for keeping you waiting.

Unfortunately, I wasn’t able to figure out a proper way to test the ImportError scenario, as I couldn’t rerun the top-level module imports reliably, even with mocking — and I’m short on time.

However, I was able to cover the pydantic v1 import and the related GenericEncoder logic, since it’s still available as a sub-package in v2. I’ve committed that test.

If you’re satisfied with this solution, please go ahead and merge the PR and create a new release. Thanks! 🙏

martincpt avatar Jun 12 '25 07:06 martincpt

Hi @Ananto30,

I’d like to get this PR finished — could we discuss how to move forward?

There are at least two issues I’ve noticed:

  1. issubclass(typ, BaseModel) raises a TypeError, which may be related to a known compatibility issue between Pydantic and Python 3.8. I’ve found a few GitHub and Stack Overflow discussions about similar errors, but it’s still unclear to me what’s causing it exactly. Interestingly, the tests pass correctly on all other Python versions. I also tried installing Python 3.8 and Pydantic locally but wasn’t able to reproduce the issue.

Would you consider dropping support for Python 3.8, given that it’s officially discontinued? See: https://devguide.python.org/versions/

What are your thoughts?

  1. The second issue occurs when running make lint. It tries to import Pydantic, which is currently not installed by default. Since it’s an extra dependency, I added it to the install-lint command. Let me know if that helps.

How do you suggest we proceed to wrap this up?

martincpt avatar Jun 27 '25 14:06 martincpt

Any thoughts @Ananto30?

martincpt avatar Jul 08 '25 13:07 martincpt

Sorry @martincpt I was caught up with many things. Let me merge this and test. I will update the tests if needed. Thanks for the good work!

Ananto30 avatar Jul 24 '25 17:07 Ananto30