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

Allow deserialization of undefined object #482

Merged
merged 12 commits into from
Feb 23, 2024
Merged

Conversation

Andy-Grigg
Copy link
Contributor

Closes #469

Handle the case where a model contains an undefined "object" and so cannot be deserialized. Add a warning for this case, and a test that checks the new behavior.

@Andy-Grigg Andy-Grigg requested a review from da1910 February 16, 2024 21:58
@github-actions github-actions bot added documentation Improvements or additions to documentation bug Something isn't working labels Feb 16, 2024
@@ -23,8 +24,9 @@
from urllib.parse import quote

import requests
from dateutil.parser import parse
from requests.structures import CaseInsensitiveDict
with warnings.catch_warnings():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was causing issues when I was trying to trap specific warnings.

"""

if data is None:
return None

if klass_name == "object":
warnings.warn("Attempting to deserialize an object with no defined type. Returning "
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to provide more information about the problematic model, but it's surprisingly hard to get to given the recursive calling.

Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.35%. Comparing base (88655dc) to head (ac7c3e8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #482      +/-   ##
==========================================
+ Coverage   94.30%   94.35%   +0.05%     
==========================================
  Files           7        7              
  Lines         773      780       +7     
==========================================
+ Hits          729      736       +7     
  Misses         44       44              

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

@Andy-Grigg
Copy link
Contributor Author

Codacy is complaining about something that black now enforces. I vote we stick with black.

@Andy-Grigg Andy-Grigg added this pull request to the merge queue Feb 23, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Feb 23, 2024
@Andy-Grigg Andy-Grigg added this pull request to the merge queue Feb 23, 2024
Merged via the queue into main with commit 3b79294 Feb 23, 2024
30 checks passed
@Andy-Grigg Andy-Grigg deleted the fix/deserialize-object branch August 21, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deserialization fails if klass_name evaluates to "object"
2 participants