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

Fix ruff errors #8936

Merged
merged 3 commits into from Aug 9, 2023
Merged

Fix ruff errors #8936

merged 3 commits into from Aug 9, 2023

Conversation

tianyizheng02
Copy link
Contributor

@tianyizheng02 tianyizheng02 commented Aug 9, 2023

Describe your change:

Fixes #8935

Fixing ruff errors again due to the recent version update

Notably, I didn't fix the ruff error in neural_network/input_data.py because it appears that the file was taken directly from TensorFlow's codebase, so I don't want to modify it just yet. Instead, I renamed it to neural_network/input_data.py_tf because it should be left out of the directory for the following reasons:

  1. Its sole purpose is to be used by neural_network/gan.py_tf, which is itself left out of the directory because of issues with TensorFlow.
  2. All of it's actually deprecated—TensorFlow explicitly says so in the code and recommends getting the necessary input data using a different function. If/when neural_network/gan.py_tf is eventually added back to the directory, its implementation should be changed to not use neural_network/input_data.py anyway.
  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the description above includes the issue number(s) with a closing keyword: "Fixes #ISSUE-NUMBER".

tianyizheng02 and others added 2 commits August 8, 2023 21:16
Renamed neural_network/input_data.py to neural_network/input_data.py_tf
because it should be left out of the directory for the following
reasons:

1. Its sole purpose is to be used by neural_network/gan.py_tf, which is
   itself left out of the directory because of issues with TensorFlow.

2. It was taken directly from TensorFlow's codebase and is actually
   already deprecated. If/when neural_network/gan.py_tf is eventually
   re-added back to the directory, its implementation should be changed
   to not use neural_network/input_data.py anyway.
@algorithms-keeper
Copy link

Multiple Pull Request Detected

@tianyizheng02, we are extremely excited that you want to submit multiple algorithms in this repository but we have a limit on how many pull request a user can keep open at a time. This is to make sure all maintainers and users focus on a limited number of pull requests at a time to maintain the quality of the code.

This pull request is being closed as the user already has an open pull request. Please focus on your previous pull request before opening another one. Thank you for your cooperation.

User opened pull requests (including this one): #8936, #8903, #8610, #8603

@algorithms-keeper algorithms-keeper bot closed this Aug 9, 2023
@algorithms-keeper algorithms-keeper bot removed the request for review from cclauss August 9, 2023 07:13
@algorithms-keeper algorithms-keeper bot added the awaiting reviews This PR is ready to be reviewed label Aug 9, 2023
@tianyizheng02 tianyizheng02 reopened this Aug 9, 2023
@algorithms-keeper
Copy link

Closing this pull request as invalid

@tianyizheng02, this pull request is being closed as the files submitted contains an invalid extension. This repository only accepts Python algorithms. Please read the Contributing guidelines first.

Invalid files in this pull request: neural_network/input_data.py_tf

@algorithms-keeper algorithms-keeper bot closed this Aug 9, 2023
@algorithms-keeper algorithms-keeper bot removed the awaiting reviews This PR is ready to be reviewed label Aug 9, 2023
@tianyizheng02
Copy link
Contributor Author

How did neural_network/gan.py_tf make it past this bot then...

Change input_data.py_tf file extension because algorithms-keeper bot is being picky about it
@tianyizheng02 tianyizheng02 reopened this Aug 9, 2023
@algorithms-keeper algorithms-keeper bot added awaiting reviews This PR is ready to be reviewed enhancement This PR modified some existing files labels Aug 9, 2023
@cclauss cclauss enabled auto-merge (squash) August 9, 2023 07:21
@cclauss
Copy link
Member

cclauss commented Aug 9, 2023

Please add an issue to rehabilitate neural_network/gan.py_tf

@cclauss
Copy link
Member

cclauss commented Aug 9, 2023

@dhruvmanila What is your take on ruff rule PYI024? This seems like a lot of rework for minimal gain. Do you believe we should ignore PYI024 or do you see sufficient value in paying attention to this rule?

@dhruvmanila
Copy link
Member

The typing.NamedTuple is the typed version of the same in collections module. I think we should move to that because it provides a lot of benefits namely types for all fields, editor awareness of the fields and IMO it has a much better API.

@cclauss cclauss merged commit ae0fc85 into TheAlgorithms:master Aug 9, 2023
5 checks passed
@algorithms-keeper algorithms-keeper bot removed the awaiting reviews This PR is ready to be reviewed label Aug 9, 2023
@dhruvmanila
Copy link
Member

@tianyizheng02 if possible, please open any enhancements or bugs related to algorithms-keeper in the repository :)

@tianyizheng02 tianyizheng02 deleted the fix-ruff branch August 9, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This PR modified some existing files invalid
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Why am i facing ruff failed on other files?
3 participants