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

Make it possible for from_diff to support custom types (issue #107) #108

Merged
merged 4 commits into from Nov 20, 2020
Merged

Make it possible for from_diff to support custom types (issue #107) #108

merged 4 commits into from Nov 20, 2020

Conversation

clj
Copy link

@clj clj commented Jan 31, 2020

This makes it possible to pass a custom dumps implementation to from_diff to support types not natively supported by Python's json module.

I have not dealt with from_string and to_string which also use the json module. The easy solution here would be to deprecate them and rely on people using whichever implementation of json dumping and loading they prefer.

@coveralls
Copy link

coveralls commented Jan 31, 2020

Coverage Status

Coverage decreased (-0.07%) to 64.914% when pulling 29c989e on paperlessreceipts:custom-types into 9e4d423 on stefankoegl:master.

@stefankoegl
Copy link
Owner

Thanks for the pull request - looks great!

Could you add a simple example to the tutorial? Thanks :)

@clj
Copy link
Author

clj commented Feb 4, 2020

I will do.

Do you have any opinion on what to do about from_string and to_string?

@stefankoegl
Copy link
Owner

I see no reason to deprecate those two. We could also add an optional parameter to those two functions.

clj added 2 commits March 6, 2020 12:07
Additionally:

* from_string gets a loads parameter
* to_string gets a dumps_parameter
* documentation added
* added more tests
@clj
Copy link
Author

clj commented Mar 6, 2020

@stefankoegl sorry about the delay in getting the PR updated, real life intervened.

I have added some documentation as requested, and used Decimal and the simplejson package as an example, as I imagine this being the primary use case for this change.

I have also made it possible to subclass JsonPatch to change its behaviour so that it is not necessary to pass dumps/loads arguments every time the individual methods are called.

Let me know if you have any comments and I'll get things turned around quicker this time :)

Copy link
Owner

@stefankoegl stefankoegl left a comment

Choose a reason for hiding this comment

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

Just one little inline comment. Everything else looks great!

jsonpatch.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Alanscut Alanscut left a comment

Choose a reason for hiding this comment

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

@stefankoegl should we merge this PR?

@stefankoegl stefankoegl merged commit 511cbc2 into stefankoegl:master Nov 20, 2020
@stefankoegl
Copy link
Owner

Merged, thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants