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

ENH: Reading large databases #266

Merged
merged 3 commits into from Mar 6, 2020
Merged

ENH: Reading large databases #266

merged 3 commits into from Mar 6, 2020

Conversation

@richardotis
Copy link
Collaborator

richardotis commented Mar 4, 2020

I noticed Database('steel_database_fix.tdb') can take a long time to complete, 40-90 seconds depending on the machine. Switching to tinydb's insert_multiple API is a cheap and easy way to cut that time in ~half.

Most of the remaining time is in parsing the grammar for all the PARAMETERs, as well as calls to sympify for the function expressions. There's room to improve there (may be possible to get the time down to ~6 seconds), but those changes may break some internal or external APIs and will be more complicated to implement. This is the low-hanging fruit.

@richardotis richardotis requested a review from bocklund Mar 4, 2020
Copy link
Collaborator

bocklund left a comment

Some thoughts in my comments. If you don't feel like anything needs to change, LGTM.

pycalphad/io/database.py Outdated Show resolved Hide resolved
pycalphad/io/database.py Outdated Show resolved Hide resolved
@bocklund

This comment has been minimized.

Copy link
Collaborator

bocklund commented Mar 5, 2020

@richardotis still want to make changes on this or is it ready?

@bocklund bocklund added this to the 0.8.2 milestone Mar 5, 2020
@bocklund bocklund merged commit 1945c55 into develop Mar 6, 2020
4 of 5 checks passed
4 of 5 checks passed
coverage/coveralls Coverage decreased (-0.0005%) to 86.005%
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@bocklund bocklund mentioned this pull request Mar 29, 2020
1 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.