search

encode / apistar

The Web API toolkit. 🛠
https://docs.apistar.com
BSD 3-Clause "New" or "Revised" License
5.57k stars 411 forks source link

python 3.7 issubclass() fix #598

Closed alkovpro closed 6 years ago

alkovpro commented 6 years ago

Hi!

I recently ran to an error trying to start my new project using python 3.7 and apistar! It appeared that this happened due to a new behavior of ABCMeta.subclasscheck and I ask you to merge this small PR, fixing issue #595

Regards, Alexander

robinchew commented 6 years ago

apistar will not run at all for Python 3.7 without this fix. Someone should merge it ASAP.

d0ugal commented 6 years ago

I just tested this, while it improve things a test still fails under 3.7. So it isn't a complete fix AFAICT.

tomchristie commented 6 years ago

This PR should also add 3.7-dev to the .travis.yml matrix . Once that's added let's take a look at the failing test noted by @d0ugal, and get that resolved too, then we can get this merged and released.

tomchristie commented 6 years ago

Ah, dammit, let's just get this in first. A PR adding 3.7-dev to the travis matrix and resolving any remaining issues would still be much appreciated.

alkovpro commented 6 years ago

Thanks) I just ran tests and there's one that failed (test_get_schema) with missing responses section in generated shema (diff screenshot attached)

test_get_schema_diff

I'll try to fix this before this weekend. Then I'll add 3.7-dev to the travis matrix and make another PR.

alkovpro commented 6 years ago

PR #612 created - problem solved

tomchristie commented 6 years ago

Is anyone able to provide a link to why 3.7 requires this __subclasscheck__ behavior?

alkovpro commented 6 years ago

I think this link refers to the case: https://www.python.org/dev/peps/pep-0560/#backwards-compatibility-and-impact-on-users-who-don-t-use-typing

tomchristie commented 6 years ago

Okay, I don't see any reference to __subclasscheck__ there. Without thinking about it too much it feels like a breaking change that made it into 3.7, but hey-ho.