+1 I’ve observed some serious differences between mongomock and pymongo that can result in some tedious debugging! I think the idea of using real Mongo is much preferred.

@munrojm @gpetretto I updated the first post on this PR with a specific request for help from each of you on one of the test failures. I'd appreciate any thoughts you have

(adding this comment b/c I'm not sure if editing the post would trigger an email notification).

@munrojm @gpetretto I updated the first post on this PR with a specific request for help from each of you on one of the test failures. I'd appreciate any thoughts you have

(adding this comment b/c I'm not sure if editing the post would trigger an email notification).

Just committed a fix for the patch test. Was an issue with how the body data was being passed to the patch method of the test client.

Thanks @rkingsbury for updating the MemoryStore. The limited number of features supported by mongomock was ineed an issue.
Concerning the test that fails, I made a few tests and figured out what is happining. The reason why I introduced those features and that test was because it was failing when the document contains a FloatWithUnit. I added that option since orjson does not handle by default subclasses of native python types, but allows to pass a function to deal with values that cannot serialize. With the mongomock implementation, when passing something like

jsonstore.update({"wrong_field": SubFloat(1.1), "task_id": 3})

the dictionary stored internally still contained an instance of SubFloat. So when update_json_file was executed orjson needed to know how to convert it. It seems that now MongoDB already stores the instance of SubFloat as a simple float. So, when it is queried back to perform update_json_file it is never an instance of SubFloat. In the new implementation I can't find a way to trigger that error anymore, as MongoDB will already deals with those cases before getting to orjson. I am not sure if it would be better to remove the serialization_default option at this point.

I added that option since orjson does not handle by default subclasses of native python types, but allows to pass a function to deal with values that cannot serialize. With the mongomock implementation, when passing something like

jsonstore.update({"wrong_field": SubFloat(1.1), "task_id": 3})

the dictionary stored internally still contained an instance of SubFloat. So when update_json_file was executed orjson needed to know how to convert it. It seems that now MongoDB already stores the instance of SubFloat as a simple float. So, when it is queried back to perform update_json_file it is never an instance of SubFloat. In the new implementation I can't find a way to trigger that error anymore, as MongoDB will already deals with those cases before getting to orjson.

Thanks for investigating! So if I understand correctly, the original problem that prompted #791 is resolved by this PR.

I am not sure if it would be better to remove the serialization_default option at this point.

But it sounds like you're saying that if I were to do this:

a = SubFloat(1.1.)
jsonstore.update({"wrong_field": SubFloat(1.1), "task_id": 3})
jsonstore.query({"task_id": 3})

I would get back a float, not a SubFloat, right? Is that a problem? It seems to me there could still be value in the options you added for users that want to be able to serialize / deserialize custom types.

Thanks for investigating! So if I understand correctly, the original problem that prompted #791 is resolved by this PR.

Indeed I think that this would have avoided the need for #791. I will try to make a test with the new version and see if there is no issue with that.

But it sounds like you're saying that if I were to do this:

a = SubFloat(1.1.)
jsonstore.update({"wrong_field": SubFloat(1.1), "task_id": 3})
jsonstore.query({"task_id": 3})

I would get back a float, not a SubFloat, right? Is that a problem? It seems to me there could still be value in the options you added for users that want to be able to serialize / deserialize custom types.

Yes, you will get back a float. A simple example is:

class SubFloat(float):
    pass

ms = MemoryStore()
ms.connect()

d = {"test": SubFloat(1.1), "normal": 1.1, "task_id": 3}
ms.update(d)

out_d = ms.query_one()
print(out_d["test"].__class__, out_d["normal"].__class__)

The output with the old version is:

<class '__main__.SubFloat'> <class 'float'>

While with the new version is:

<class 'float'> <class 'float'>

In the end it is not really a problem, since this makes the MemoryStore more similar to the MongoStore. However, any object that is not handled by jsanitize (e.g. MSONable, dataclasses, ...) will probably be converted to a simple type during the update() (a string in the worst case). So I don't know if there is any corner case where the object survives after the insertion in MongoDB, but that cannot be handled by orjson by default. I suppose leaving the option will not do much harm (at least will not force me to make a backward incompatible change to use the latest version of maggma), however I couldn't find a way to create a test example to replace the test_jsonstore_orjson_options.