search

daggaz / json-stream

Simple streaming JSON parser and encoder.
MIT License
122 stars 18 forks source link

Implement Sequence and Mapping ABC Methods #52

Open greateggsgreg opened 1 year ago

greateggsgreg commented 1 year ago

This PR is a companion to #50, since issubclass() and isinstance() should imply that all of the ABC's methods are implemented.

With the exception of index() and __contains__(), the methods added to the persistent classes require reading the entire object anyway, so I just use self.read_all(). As I was posting this, I realized that __contains__() can be chained, not requiring the entire object to be read. I will modify that method when I revisit this.

The methods added to the transient classes follow the pattern of using as little memory as possible, but in the case of __reversed__(), I didn't see a way to avoid having the entire object in memory at first.

The index() method is mostly implemented in the list base class, with the child methods simply handling the offset math.

I do have tests, but they need refactoring before I push them. I will be out of town until next Monday, but I wanted to push what I had to see if you had any initial feedback.

daggaz commented 1 year ago

Hmm, interesting point on transient __reversed__(), I'm tempted to say we should just raise NotImplementedError, perhaps with a helpful message indicating that you should switch to persistent types for this part or first convert to standard types, or something?

Maybe a new section in the docs with the exception linking to there?