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

Add Linter and Formatter for python source codes #1601

Closed
anencore94 opened this issue Aug 1, 2021 · 16 comments
Closed

Add Linter and Formatter for python source codes #1601

anencore94 opened this issue Aug 1, 2021 · 16 comments
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed kind/feature lifecycle/frozen

Comments

@anencore94
Copy link
Member

/kind feature

Describe the solution you'd like

In kubeflow/katib, there are lots of source codes written in python. Even though there are well-defined CI processes for source codes written in Golang, however for Python codes, only charmed-katib seems to have such process now.

There should be a fixed linter, formatter and tester with some rule-config files(.flake8, .pylintrc, ...) and the github action workflow to check those all. For example, there are lots of unit test codes for suggestion, but current CI process doesn't check if they are failed or check the test coverage decreased in every PR.
If there are any convention checking tools, please share in katib community :)

Anything else you would like to add:
If you guys agree to this, I'd like to contribute to this feature in some parts.

@andreyvelich
Copy link
Member

andreyvelich commented Aug 2, 2021

That is a great suggestion and it will be very useful for us.
Thank you for your help @anencore94!

In addition to that, I think we should move our current Python tests from AWS CI Workflow to GitHub actions: https://github.com/kubeflow/katib/blob/master/test/scripts/v1beta1/python-tests.sh.

Currently, we are not using AWS Kubernetes cluster requirement to perform this workflow step.
We can create separate GitHub Actions Workflow for all Python tests (linter, formatter, unit), similar to Go: https://github.com/kubeflow/katib/tree/master/.github/workflows.

cc @gaocegege @johnugeorge

@anencore94
Copy link
Member Author

Using mypy to type check would be nice too.

@gaocegege
Copy link
Member

SGTM, I am not quite familiar with Python, I think we need it.

@stale
Copy link

stale bot commented Jan 3, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale label Jan 3, 2022
@andreyvelich
Copy link
Member

/lifecycle frozen

@tenzen-y
Copy link
Member

/good-first-issue

Copy link

@tenzen-y:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@google-oss-prow google-oss-prow bot added good first issue Good for newcomers help wanted Extra attention is needed labels Mar 11, 2024
@sagnik3788
Copy link

@sandipanpanda still working on it ?

@sandipanpanda
Copy link
Member

Sorry, this fell off my radar. @sagnik3788 feel free to work on this.
/unassign

@sagnik3788
Copy link

/assign

@forsaken628
Copy link
Contributor

Which formater is better, black, autopep8 or something else?

@tenzen-y
Copy link
Member

Which formater is better, black, autopep8 or something else?

We are supposed to use the flake8 and black.

@Souradip121
Copy link

@sagnik3788 still working on it??

@tenzen-y
Copy link
Member

This has already been fixed by #2407.
/close

/assign @Ygnas

Copy link

@tenzen-y: GitHub didn't allow me to assign the following users: Ygnas.

Note that only kubeflow members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

This has already been fixed by #2407.
/close

/assign @Ygnas

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

@tenzen-y: Closing this issue.

In response to this:

This has already been fixed by #2407.
/close

/assign @Ygnas

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed kind/feature lifecycle/frozen
Projects
None yet
Development

No branches or pull requests

9 participants