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

Deprecate Native API #1143

Closed
wants to merge 206 commits into from
Closed

Conversation

tanwarsh
Copy link
Collaborator

@tanwarsh tanwarsh commented Nov 13, 2024

Deprecate Python API:

  1. Mark Python native API as deprecated in the doc
  2. Remove Python native API code.
  3. Remove github workflow realted code.
  4. Remove test related to Python native API.
  5. Remove Tutorials related to Python native API.

This PR will be merged after Interactive API Deprecation #1125

@tanwarsh tanwarsh marked this pull request as draft November 13, 2024 06:27
@tanwarsh tanwarsh marked this pull request as ready for review November 13, 2024 06:38
Signed-off-by: Shah, Karan <[email protected]>
Signed-off-by: Shah, Karan <[email protected]>
@tanwarsh tanwarsh changed the title WIP: Deprecate Python API Deprecate Python API Nov 13, 2024
Copy link
Collaborator

@teoparvanov teoparvanov left a comment

Choose a reason for hiding this comment

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

LGTM for a first pass (assuming deletion of the Python Native API, rather than deprecation). Could you make sure that the CI passes, so that we can have a good understanding of the entire impact of the deletion?

docs/developer_guide/advanced_topics/overriding_agg_fn.rst Outdated Show resolved Hide resolved
@@ -4,7 +4,7 @@
.. _running_notebook:

**********************************
Aggregator-Based Workflow Tutorial
Aggregator-Based Workflow Tutorial (Deprecated)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't related to Python API, so please revert this change

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@teoparvanov, I think we should rewrite/delete this as the title is Aggregator based but the the tutorial is using python native API.
https://openfl.readthedocs.io/en/latest/developer_guide/running_the_federation.notebook.html

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then my suggestion is to delete the entire page.

docs/get_started/examples.rst Outdated Show resolved Hide resolved
docs/developer_guide/utilities/splitters_data.rst Outdated Show resolved Hide resolved
docs/get_started/examples/python_native_pytorch_mnist.rst Outdated Show resolved Hide resolved
docs/source/api/openfl_native.rst Outdated Show resolved Hide resolved
tanwarsh and others added 26 commits November 17, 2024 20:46
Signed-off-by: noopur <[email protected]>
@MasterSkepticista
Copy link
Collaborator

@tanwarsh Because the tip has moved way ahead, a better option is to create patches of your changes, close this PR, and create a new branch from develop with your patches applied.

@tanwarsh
Copy link
Collaborator Author

Closing this PR due to rebase issues. Create new PR #1153 for the same.

@tanwarsh tanwarsh closed this Nov 18, 2024
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.