F/remove sklarn #49
Merged
F/remove sklarn #49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
wooohoooo
commented
May 6, 2022
wooohoooo
commented
- removed dependency on Sklearn etc, in setup.py and all the places where I could find it
- removed run() and other pieces of the API that aren't supposed to be used any more
- made sure the tests cover the use cases
- added notebook that showcases the use of the package (can be used as secondary testing)
added 9 commits
February 3, 2022 09:19
…ved sklearn imports from sclblpy.__init__
…ittests or pytest standards?
…e c) device registered
ayoub-assis
reviewed
May 9, 2022
README.md
Outdated
| @@ -3,8 +3,7 @@ | |||
| [](https://pypi.org/project/sclblpy/) | |||
|
|
|||
|
|
|||
| Changelog: the package is currently undergoing a major rework. We try to keep the below up to date, but some features might be stubs. | |||
| Most relevant is the removal of support for Scikit learn in favor of focussing fully on ONNX. | |||
| Changelog: the package is currently being refactored. As such, it is currently a stub, aas Scailable has changed its focus from Scikit Learn to ONNX, Scikit Learn support has been removed. | |||
ayoub-assis
reviewed
May 9, 2022
test/test_main.py
Outdated
|
|
||
| print("Wait, toolchain needs to finish....") | ||
| time.sleep(10) | ||
| time.sleep(25) | ||
| print(" waited for 10 seconds") |
There was a problem hiding this comment.
10 should be updated to 25
|
Everything looks fine to me. |
|
You should definitely setup your own account and create a device so you can test!
… On 10 May 2022, at 09:56, ayoub-assis ***@***.***> wrote:
Everything looks fine to me.
Albeit I couldn't run the notebook and test files since I don't have an account with a device.
But maybe that's something that is irrelevant to this PR.
@robinvanemden <https://github.com/robinvanemden>
—
Reply to this email directly, view it on GitHub <#49 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABGHUD44BM42NMX2YSK3TT3VJIJDTANCNFSM5VIG7IVA>.
You are receiving this because you are subscribed to this thread.
|
ayoub-assis
reviewed
May 10, 2022
|
|
||
| ## Background | ||
| The sclblpy package allows users with a valid scailable account (get one at [https://admin.sclbl.net/signup.html](https://admin.sclbl.net/signup.html)) | ||
| to upload fitted ML / AI models to the Scailable toolchain server. This will result in: | ||
|
|
||
| 1. The model being tested for errors on the client side. | ||
| 2. The model being uploaded to Scailable, tested again, and if all test pass it will be converted to [WebAssembly](https://webassembly.org). | ||
| 3. The model being made available to deploy to a pre-regsitered (link on how to do that) device | ||
| 3. The model being made available to deploy to a preregistered (link on how to do that) device | ||
| 4. (upcoming) The model being testable through test_run(example_input, device_id) | ||
|
|
||
| ## Getting started |
ayoub-assis
reviewed
May 10, 2022
ayoub-assis
reviewed
May 10, 2022
README.md
Outdated
| @@ -178,7 +177,7 @@ the following packages: | |||
| * `sklearn` (legacy, to be removed in future updates) | |||
There was a problem hiding this comment.
sklearn is no longer a package dependency
ayoub-assis
reviewed
May 10, 2022
test/test_main.py
Outdated
|
|
||
|
|
||
| def test_upload(): | ||
| def test_upload_onnx(): | ||
| """ Test the upload function (sklearn and onnx""" |
There was a problem hiding this comment.
Here Test the upload function sklearn and onnx
ayoub-assis
reviewed
May 10, 2022
test/test_main.py
Outdated
|
|
||
| # ONNX | ||
| docs['name'] = "Name of ONNX model" | ||
| docs['documentation'] = "A long .md thing...." | ||
| check = upload("../test/files/model.onnx", "", docs, model_type="onnx") | ||
| check = upload_onnx("../test/files/model.onnx", "", docs) | ||
| assert check is True, "ONNX upload test failed." | ||
|
|
||
|
|
||
| def test_update(): | ||
| """ Test the update function (sklearn & onnx""" |
There was a problem hiding this comment.
Here Test the upload function (sklearn and onnx
|
ayoub-assis
reviewed
May 10, 2022
test/test_main.py
Outdated
| @@ -258,16 +187,31 @@ def test_user_utils(): | |||
| print("Running simple functional tests of main.py") | |||
| print("===============================") | |||
|
|
|||
| test_upload() # upload sklearn and onnx test | |||
| test_upload_onnx() # upload sklearn and onnx test | |||
There was a problem hiding this comment.
upload sklearn and onnx test ---> upload onnx test
ayoub-assis
reviewed
May 10, 2022
test/test_main.py
Outdated
|
|
||
| print('testing update \n\n') | ||
| test_update() # update sklearn and onnx test |
There was a problem hiding this comment.
update sklearn and onnx test --> update onnx test
Manual Testing
|
|
|
Works for me locally. All issues that were found were also resolved by @ayoub-assis . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.