Skip to content

No typing on bytestream attributes - #132

Closed
lovelydinosaur wants to merge 1 commit into
masterfrom
no-typing-on-bytestream-attributes
Closed

No typing on bytestream attributes#132
lovelydinosaur wants to merge 1 commit into
masterfrom
no-typing-on-bytestream-attributes

Conversation

@lovelydinosaur

Copy link
Copy Markdown
Contributor

We need to do a bit of a dance on AsyncByteStream/SyncByteStream, since we're using them both as an interface class, and include a plain base implementation for simple usages.

By using setattr/getattr instead of typical attribute setting we can avoid type checkers making assumptions about how what types base classes should expect on attributes they might use, like .content.

@lovelydinosaurlovelydinosaur added this to the 0.10.0 milestone Aug 3, 2020
@florimondmanca

Copy link
Copy Markdown
Contributor

Hmm, not sure about this.

This could be a smell that we should really separate the interface from the handy basic implementation.

But at the same time I don't know how I'd call things if they had to be split apart, so...

What's your feeling about this @tomchristie?

@lovelydinosaur

Copy link
Copy Markdown
ContributorAuthor

That's a great observation. We could keep the interfaces pure, and provide SimpleByteStream, IteratorByteStream, and AiteratorByteStream concrete implementations.

@florimondmanca

Copy link
Copy Markdown
Contributor

Yes, that's mostly the alternative I was thinking about - dedicated implementations for each.

@lovelydinosaur

Copy link
Copy Markdown
ContributorAuthor

Closing in favour of #133

@lovelydinosaur
lovelydinosaur deleted the no-typing-on-bytestream-attributes branch August 5, 2020 10:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@lovelydinosaur@florimondmanca