Extensibility of [Relative]JSONPointer classes?

[handrews]

@marksparkza in the OAI/oascomply project, I find myself subclassing jschon.JSONPointer and jschon.RelativeJSONPointer for smoother integration with other aspects of that project. This includes raising a subclass of jschon.JSONPointerError or jschon.RelativeJSONPointerError that also subclasses the root of oascomply's exception hierarchy.

This is, perhaps, a rather unusual use case, but it could be made easier and more performant with some very small changes:

• when returning new instances, replace [Relative]JSONPointer(...) with type(self)(...) (or similar, if there's a better way to do that in Python3
• allow specifying a subclass of [Relative]JSONPointerError as a constructor parameter, and raising it in place of the hardcoded exception classes

I don't think this would require changing type hints since this involves subclassing rather than defining a protocol - in this use case, I want jschon code to treat my subclasses just as if they were the jschon-provided classes.

Is this something you would be interested in as a PR, or do you want to exclude this use case? I'll happily work with either, I just figured it was worth asking before investing in working around the current design.

[marksparkza]

Sounds reasonable; I assume you just want to change pointer instantiation in the jsonpointer module itself? self.__class__ will work, or just cls in a class method.

On raising exception subclasses, I think rather than changing constructor signatures this could be encapsulated by the associated pointer classes, for example via an exc_class class attribute.

[handrews]

I assume you just want to change pointer instantiation in the jsonpointer module itself? self.class will work, or just cls in a class method.

Yes. The idea of a more comprehensive extensibility shift, where (for example) the JSON class or its subclasses might instantiate a corresponding JSONPointer subclass, is interesting in the context of what I'm doing to try to support OpenAPI 3.x, but that would be a substantially more complex undertaking and I am not certain that it would be the right way to go about it.

Making this change just within the jschon.jsonpointer module (and ideally #101 or something similar as discussed in #99 (comment)) would help me balance performance and design consistency while I work on the best way to structure OpenAPI support. #99 is probably a better place to discuss extensibility of the JSON class.

On raising exception subclasses, I think rather than changing constructor signatures this could be encapsulated by the associated pointer classes, for example via an exc_class class attribute.

Yes, that makes more sense, I'm not sure what I was thinking with the constructor thing.

It sounds like it's worth putting up a PR for this direction, so I'll do that :-)

[handrews]

@marksparkza

I forgot to bring up one thing: Sometimes I need to distinguish between errors during construction of a pointer and during evaluation. This is easy if handling errors close to the source, but if farther away there's sometimes a difference between handling or reporting a malformed pointer vs being unable to resolve a well-formed pointer with a particular JSON document. I usually do this with small exception class hierarchies to avoid having to regex match the error message. I see three possibilities here:

1. Stick with one cls.exc_class each for JSONPointer and RelativeJSONPointer – this is the minimal change and I'm happy to do it if that's what you prefer. I'm not trying to force a philosophical change in exception design
2. Have two fields, cls.construction_exc_class and cls.evaluation_exc_class or similar, for each of the two classes, but set them both to the same thing in the jschon classes – this would preserve the existing behavior in jschon but allow a different exception design in subsclasses
3. Have two fields and create separate exception classes within jschon

I would prefer option 2, but would be perfectly happy with option 1. These classes are small enough that changes in the error messages are relatively rare, and there aren't that many cases. I'd also happily do option 3, of course, but it feels like making a change to your interface for what are essentially stylistic preferences.

[marksparkza]

Option 3 😁

I've just released 0.11 which should provide a better starting point for your proposed changes. You can now distinguish between construction and evaluation errors. I'd suggest malformed_exc and reference_exc as the names of the overridable class attributes.

[handrews]

Woo hoo! 🙏 🚀

[marksparkza]

@handrews I'm reopening this issue, as #104 only covered the exception subclassing part; dynamic instantiation of [Relative]JSONPointer must still be implemented.

[handrews]

@marksparkza 🤦 I'll get to that... it might take until the week after next

[marksparkza]

Cool, no rush. Also, I've not forgotten all your other PRs! From mid-July I will have time to start looking at the more complex and interrelated ones.