-
Notifications
You must be signed in to change notification settings - Fork 3
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
63 add database logging functionality #84
Conversation
c23c6f4
to
d4002e5
Compare
d4002e5
to
7ae7f8b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- There are warnings in the mysql view of all tables, which should be removed/solved.
- The LogTables are missing the timestamp column as noted in the issue
- The order of the columns should be:
- ID
- experiment_id
- timestamp
- other columns in order of the config file
7ae7f8b
to
6f83802
Compare
Now the functions are ordered by importance.
Logtables now get created in 'create_table_if_not_existing'. Additionally the code for creating tables was heavily reworked.
6f83802
to
9acddc4
Compare
Move timestamp handling to database_connector whenever possible. Introduce utils.get_timestamp_representation to remove duplicated code.
These warnings seem to get caused by the database connection to a 'modern' database. Therefore the comment should be moved to #61 . |
The dependency is used for several tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Update documentation
So I tried using the new functionality in one of the examples and I have two problems with running the examples in the first place:
|
This is updated
I can not reproduce this error. |
93afbd5
to
0560ee4
Compare
5f0d299
to
ee57a23
Compare
I also added documentation and an example notebook. Note that this branch is checked out before #87 is merged into develop. Therefore, we must manually add string identifiers when writing a string into a log-table. This should be fixed by closing the aforementioned issue |
I retried running the examples and it still fails for me. Steps to reproduce on a MAC with Mac OS Monterey (12.6):
I shortened some of the paths in the above error. Edit: I also get the same error when running it outside of Pycharm. |
T
This prompted us to use joblib instead of pytons own multiprocessing. The issue #102 deals with this. It is finished but needs to merged on dev. |
With the fixes from the other branch and the adapted usage and example documentation, it looks good to me now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.