Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: callables can be deserialized from fully qualified import path #8788

Merged
merged 9 commits into from
Feb 3, 2025

Conversation

mathislucka
Copy link
Member

Related Issues

Proposed Changes:

How did you test it?

Notes for the reviewer

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I documented my code
  • I ran pre-commit hooks and fixed any issue

@mathislucka mathislucka requested a review from a team as a code owner January 30, 2025 16:52
@mathislucka mathislucka requested review from mpangrazzi and removed request for a team January 30, 2025 16:52
@github-actions github-actions bot added the type:documentation Improvements on the docs label Jan 30, 2025
@mathislucka mathislucka added the ignore-for-release-notes PRs with this flag won't be included in the release notes. label Jan 30, 2025
@coveralls
Copy link
Collaborator

coveralls commented Jan 30, 2025

Pull Request Test Coverage Report for Build 13109457248

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 9 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.01%) to 91.362%

Files with Coverage Reduction New Missed Lines %
utils/callable_serialization.py 1 95.35%
dataclasses/answer.py 2 97.94%
dataclasses/document.py 6 93.52%
Totals Coverage Status
Change from base Build 13070083220: 0.01%
Covered Lines: 8874
Relevant Lines: 9713

💛 - Coveralls

Copy link
Contributor

@mpangrazzi mpangrazzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I've only left two comments.

@mpangrazzi
Copy link
Contributor

@mathislucka one last thing: it would also make sense to add a release note file. WDYT?

@mathislucka mathislucka requested a review from a team as a code owner February 3, 2025 08:37
@mathislucka mathislucka requested review from dfokina and removed request for a team February 3, 2025 08:37
@mathislucka mathislucka removed the ignore-for-release-notes PRs with this flag won't be included in the release notes. label Feb 3, 2025
@mathislucka
Copy link
Member Author

Should be ready to go now.

@mpangrazzi mpangrazzi self-requested a review February 3, 2025 11:00
@mathislucka mathislucka merged commit 1a91365 into main Feb 3, 2025
20 checks passed
@mathislucka mathislucka deleted the fix/fully_qualified_callable_serde branch February 3, 2025 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:tests type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deserialize_callable does not work with all fully qualified import paths
3 participants