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

test: algo to function in substratools #303

Merged
merged 13 commits into from
Oct 11, 2022
Merged

Conversation

ThibaultFy
Copy link
Member

@ThibaultFy ThibaultFy commented Oct 6, 2022

Signed-off-by: ThibaultFy 50656860+ThibaultFy@users.noreply.github.com

Companion PR:

Related issue

# followed by the number of the issue

Summary

Notes

Please check if the PR fulfills these requirements

  • If necessary, the changelog has been updated
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • The commit message follows the conventional commit specification
  • For any breaking changes, companion PRs have been opened on the following repositories:

@ThibaultFy ThibaultFy marked this pull request as ready for review October 7, 2022 07:02
@ThibaultFy ThibaultFy marked this pull request as draft October 7, 2022 07:05
@ThibaultFy ThibaultFy force-pushed the gt/class_to_function branch from 9174ada to 8456ec1 Compare October 7, 2022 07:33
@ThibaultFy ThibaultFy changed the title test: from algo class to function on substratools test: algo to function in substratools Oct 7, 2022
@ThibaultFy ThibaultFy marked this pull request as ready for review October 7, 2022 07:34
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
@ThibaultFy ThibaultFy force-pushed the gt/class_to_function branch from 29bce2f to 00835a2 Compare October 7, 2022 11:46
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
CHANGELOG.md Outdated
@@ -98,7 +102,7 @@ compute_plan = client.add_compute_plan(
- used openers within the library only exposes `get_data` and `fake_data` methods due to breaking changes within
substra-tools
- a third argument `task_properties` (containing the `rank` of a task) has been added to all algo methods relying on substra-tools
- substra tools metrics `tools.Metrics` become `tools.MetricAlgo` and `tools.metrics.execute` becomes `tools.algo.execute` (#290)
- substra tools metrics `tools.Metrics` become `tools.MetricAlgo` and `tools.metrics.execute` becomes `tools.execute` (#290)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better to add a line in unreleased as it was released that way in 0.39

DEFAULT_SUBSTRATOOLS_VERSION = (
f"latest-nvidiacuda11.6.0-base-ubuntu20.04-python{sys.version_info.major}.{sys.version_info.minor}-minimal"
)
DEFAULT_SUBSTRATOOLS_VERSION = "class-to-function" # TODO: change before merge
Copy link
Contributor

@Fabien-GELUS Fabien-GELUS Oct 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • .

@@ -73,8 +73,8 @@ def test_command_config(workdir):
assert list(cfg.keys()) == expected_profiles


def mock_client_call(mocker, method_name, response="", side_effect=None):
return mocker.patch(f"substra.cli.interface.Client.{method_name}", return_value=response, side_effect=side_effect)
def mock_client_call(mocker, function_name, response="", side_effect=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is not related to substratools

@Fabien-GELUS Fabien-GELUS self-requested a review October 7, 2022 14:05
CHANGELOG.md Outdated
@@ -98,7 +102,7 @@ compute_plan = client.add_compute_plan(
- used openers within the library only exposes `get_data` and `fake_data` methods due to breaking changes within
substra-tools
- a third argument `task_properties` (containing the `rank` of a task) has been added to all algo methods relying on substra-tools
- substra tools metrics `tools.Metrics` become `tools.MetricAlgo` and `tools.metrics.execute` becomes `tools.algo.execute` (#290)
- substra tools metrics `tools.Metrics` become `tools.MetricAlgo` and `tools.metrics.execute` becomes `tools.execute` (#290)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not change the previous changelog entries

CHANGELOG.md Outdated
@@ -535,5 +539,4 @@ client1_org_id = clients[0].organization_info().organization_id

## [0.10.0](https://github.com/Substra/substra/releases/tag/0.10.0) - 2021-08-05

[0.10.0]: https://github.com/Substra/substra/compare/0.9.0...0.10.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why change this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's deleted by my autosave lint... Don't know why sorry about that

Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com>
@ThibaultFy ThibaultFy merged commit 15f7e1c into main Oct 11, 2022
@ThibaultFy ThibaultFy deleted the gt/class_to_function branch October 11, 2022 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants