Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DO NOT MERGE] Drop py35 #190

Merged
merged 13 commits into from
Sep 24, 2019
Merged

[DO NOT MERGE] Drop py35 #190

merged 13 commits into from
Sep 24, 2019

Conversation

grigi
Copy link
Member

@grigi grigi commented Sep 13, 2019

The plan is to have this as the next master base. I'm leaving this here as a PR so its easier to manage.

DONE:

  • Drop Py3.5 support
  • Remove py3.5 "hacks" in tests/examples.
  • Upgrade to f-strings (used flynt and pyupgrade to help automate)
  • Use variable annotations
  • Update docs re py3.6 now being minimal supported version.
  • See if it is beneficial to use async generators/comprehensions in core code
  • OptimizeModel._init_from_db to minimize calling to_python_value if we have guarantees that allow for it to be skipped.

@coveralls
Copy link

coveralls commented Sep 13, 2019

Pull Request Test Coverage Report for Build 797

  • 261 of 266 (98.12%) changed or added relevant lines in 23 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 98.263%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tortoise/models.py 70 72 97.22%
tortoise/queryset.py 41 44 93.18%
Totals Coverage Status
Change from base Build 793: 0.2%
Covered Lines: 2858
Relevant Lines: 2886

💛 - Coveralls

@grigi
Copy link
Member Author

grigi commented Sep 20, 2019

@abondar This PR has two basic things:

  1. Changed to Py3.6+ syntax & features. This should actually reduce the line count noticeably.
  2. Added a lot of tests, and fixed a few minor bugs.

I am going to split this up into the tests & bugfixes (backport for 0.13.6).
Then rebase this to ONLY do the py3.6+ conversion.

But please have a look at this PR :-)

@grigi grigi mentioned this pull request Sep 21, 2019
@grigi grigi changed the base branch from master to develop September 21, 2019 17:30
@abondar
Copy link
Member

abondar commented Sep 23, 2019

Looks all good to me
Am I right that speedup was achieved by implementing all things around native fields?

@grigi
Copy link
Member Author

grigi commented Sep 23, 2019

Yes, the speedup was mainly on two "special" cases:

  1. Native fields, as in types supported natively by the DB/connector/Python, requires no conversion.
  2. When we use the default to_python converter, implement the code in a loop instead of calling the method. In my profiling I found that the method call was quite high, as it did virtual table lookups, and all that the default to_python method was was to «cast» or None which was a lot less than the mthod call overhead.

This gave a better speedup with larger models with more fields.

We could probably do a similar speedup for creating from Python via the constructor, and the converting to the DB fields. But we are running out of Python-level inefficiencies to optimize. We'll soon have to expand the benchmarking system to test DB/connector/Tortoise interactions. e.g. Are we using the indexes correctly? Is there any unnecessary overhead with the DB connectors for MySQL/PostgreSQL? Do we set up the DB fields correctly for case-insensitive operations? etc...

@grigi grigi merged commit a59e21b into develop Sep 24, 2019
@grigi grigi deleted the drop_py35 branch September 24, 2019 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants