chovanecm / sacredboard

Dashboard for sacred. Monitor and access your past machine learning experiments.
MIT License
184 stars 39 forks source link

abstract data interface #43

Closed gideonite closed 7 years ago

gideonite commented 7 years ago

Abstract the interface for data storage from anything mongodb specific, thereby allowing for developers to more easily incorporate other backends such as file storage and tinydb.

Basic implementation for file storage with corresponding unit tests.

For more on this change see https://github.com/chovanecm/sacredboard/issues/17.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+3.05%) to 74.05% when pulling 5ecdcf4015f828c80b662788cb3d7207bb21a855 on gideonite:develop into 45e9b73f0659ba724b682fa783d12e443381c003 on chovanecm:develop.

gideonite commented 7 years ago

I'd be more than happy to but I don't know how to run the linter tool.

I'm pretty sure allow edits from maintainers is already on:

image

coveralls commented 7 years ago

Coverage Status

Coverage increased (+3.05%) to 74.05% when pulling ecc2caa96f7901a0196d06781bc14bfb480aab3e on gideonite:develop into 45e9b73f0659ba724b682fa783d12e443381c003 on chovanecm:develop.

chovanecm commented 7 years ago

If you still have troubles running tox (e.g. tox -e flake8), the commands for running it are defined in tox.ini. So, for instance, flake8 sacredboard --max-complexity 10 --exclude tests checks the code style and pydocstyle sacredboard --ignore D207,D212,D301,D203 the Python code documentation. This is known to work on Windows. Tox, unfortunately, doesn't run the second command if the first one fails, so it doesn't say anything about your documentation style if there is a problem with code style. I have to resolve it somehow, perhaps I will have to add a Makefile, but it won't work for most users on Windows either.

Also make sure to have both flake8 and pydocstyle installed (I just noticed I forgot to add them in dev-requirements.txt, so if you download the current version and run pip install -r dev-requirements.txt, it should install automatically, or simply run pip install flake8 pydocstyle).

gideonite commented 7 years ago

Whoops, I guess I should have pulled before I pushed. Should I pull and resolve the changes or leave that to you? The only changes I made to mongodb in this last commit relate to removing whitespace and shortening doc strings as recommended by flake8.

chovanecm commented 7 years ago

Conflict fixed. I have also fixed some errors and added reading of the info.json file.

Furthermore, I changed the way how the code style check for Python works. Now, tox -e flake8 checks the code style whereas tox -e pydocstyle is responsible for code documentation checks. The reason for it is that when flake8 found errors, pydocstyle did not run at all.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+3.5%) to 74.513% when pulling c8f3c88d88071372863161b4cbad6f6878b37b56 on gideonite:develop into 53ffcb40671783b93c0242d6c1d464fd7b59f4df on chovanecm:develop.

chovanecm commented 7 years ago

Now I'd like to ask you to consult the code documentation with tox -e pydocstyle (of course, only files that are relevant to your part).

Do you plan to add support for filtering, sorting etc? It's quite a lot of work. Otherwise, I'm thinking about extending the Web API to tell the frontend to hide the filtering and similar controls in order not to confuse the users. Also, some error handling (file not found) etc would be nice. But for that, I have to modify the Web API.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+3.5%) to 74.513% when pulling ec5633978f3a2cbfc9223c72e511a53ddd73084d on gideonite:develop into 53ffcb40671783b93c0242d6c1d464fd7b59f4df on chovanecm:develop.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+3.3%) to 74.313% when pulling 804b5637437348af50458faf3d4c90f18c643ef5 on gideonite:develop into 53ffcb40671783b93c0242d6c1d464fd7b59f4df on chovanecm:develop.

chovanecm commented 7 years ago

"Why is m or mu compared to (None, None)?" - It's not exactly clear, I know. The reason is that the mu parameter defaults to (None, None) and when the parameter is not present on the command line, it is set to that value. So after you added "if m or mu", it was actually always evaluated as True even thought Sacredboard was run with -F. I have to refactor the bootstrap. It's getting a little bit messy.

gideonite commented 7 years ago

Oh ok, sorry about that. I should have tested it more thoroughly.

Any other things that this PR is blocked on?

chovanecm commented 7 years ago

Not from your side. As soon as I have some time (hopefully already later this week), I'll disable all the sorting and filtering controls for backends that don't support it and I'll merge the PR.