[ENH] V1 → V2 API Migration - setups#1619
[ENH] V1 → V2 API Migration - setups#1619EmanAbdelhaleem wants to merge 77 commits intoopenml:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1619 +/- ##
==========================================
+ Coverage 52.04% 54.46% +2.42%
==========================================
Files 36 63 +27
Lines 4333 5064 +731
==========================================
+ Hits 2255 2758 +503
- Misses 2078 2306 +228 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Running the tests for both files: To reproduce: |
This reverts commit fd43c48.
|
@geetu040 Ready for review |
geetu040
left a comment
geetu040
left a comment
There was a problem hiding this comment.
https clients are already initialized, you copy design from tests/test_api/test_versions.py
| flow.name = f"{get_sentinel()}{flow.name}" | ||
| flow.publish() | ||
| TestBase._mark_entity_for_removal("flow", flow.flow_id, flow.name) | ||
| TestBase.logger.info(f"collected from {__file__.split('/')[-1]}: {flow.flow_id}") |
There was a problem hiding this comment.
it's nice that you have it here, but it won't take affect, I should add TestBase as parent for TestAPIBase so this works
| assert setup_id == run.setup_id | ||
|
|
||
|
|
||
| class TestSetupV2(TestAPIBase): |
There was a problem hiding this comment.
this class has no tests, you should check if it not implemented methods raise the correct exception
| self.resource = SetupV1API(self.http_client) | ||
| self.extension = SklearnExtension() | ||
|
|
||
| @pytest.mark.uses_test_server() |
There was a problem hiding this comment.
can be moved to class level
There was a problem hiding this comment.
You are talking about the extension, right?
I followed the code of the existing tests. and it's initialized in the setUp() function, I am not even sure if moving it would make difference, the setUp() is already called before running the class tests, rigth?
There was a problem hiding this comment.
good, hopefully other tests are not breaking.
5 participants
Fixes #1625
Depends on #1576
Related to #1575
Details
This PR implements
Setupsresource, and refactor its existing functions