Skip to content

data_structures/linked_list: Adding __str__() function #3960 #3961

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

Merged
merged 10 commits into from
Nov 28, 2020

Conversation

arif599
Copy link
Contributor

@arif599 arif599 commented Nov 26, 2020

Describe your change:

Added a str() function to the Linked List class that will print the contents in the Linked List with arrows that point to the next value

see: #3960

  • 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 have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

@ghost ghost added awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass and removed awaiting reviews This PR is ready to be reviewed labels Nov 26, 2020
@arif599 arif599 changed the title data_structures/linked_list: Add __str__() function #3960 data_structures/linked_list: Adding __str__() function #3960 Nov 26, 2020
@arif599
Copy link
Contributor Author

arif599 commented Nov 26, 2020

I'm not really sure what went wrong during the build check.

@amaank404
Copy link
Contributor

Your doctest failed, please verify it

@arif599
Copy link
Contributor Author

arif599 commented Nov 26, 2020

Thanks for bringing that up. I've never used doctests in python before so I'll do some more research. Based upon looking at other Pull Requests, I thought it was just a comment that shows others how to use it but I didn't know it also gets executed.

Copy link
Contributor

@amaank404 amaank404 left a comment

Choose a reason for hiding this comment

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

Will fix any doctest errors, make the output better to look at without any extra arrow at the end

also a spelling error iteam -> item

@ghost ghost added awaiting reviews This PR is ready to be reviewed and removed tests are failing Do not merge until tests pass labels Nov 26, 2020
@arif599
Copy link
Contributor Author

arif599 commented Nov 26, 2020

Will fix any doctest errors, make the output better to look at without any extra arrow at the end

also a spelling error iteam -> item

Thank you for catching that spelling mistake. I really like the format of the output, I committed the changes you added.

Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

If possible, can you add type hints for other functions/parameters as well? Thanks if you do, otherwise after this small change the PR is good to go.

@ghost ghost added awaiting changes A maintainer has requested changes to this PR and removed awaiting reviews This PR is ready to be reviewed labels Nov 26, 2020
@arif599
Copy link
Contributor Author

arif599 commented Nov 26, 2020

Yeah definitely, I'll include type hinting and also add comments to describe some of the functionality. It's midnight for me, so I will have it done by tomorrow morning.

@amaank404
Copy link
Contributor

Great, Also your doctests were failing because of the whitespace in that line which was required but trimmed by pre commit

@ghost ghost added awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass and removed awaiting changes A maintainer has requested changes to this PR awaiting reviews This PR is ready to be reviewed labels Nov 26, 2020
@ghost ghost added awaiting reviews This PR is ready to be reviewed and removed tests are failing Do not merge until tests pass labels Nov 26, 2020
@arif599 arif599 requested a review from dhruvmanila November 26, 2020 20:53
@ghost ghost added awaiting changes A maintainer has requested changes to this PR and removed awaiting reviews This PR is ready to be reviewed labels Nov 27, 2020
@ghost ghost added awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass and removed awaiting changes A maintainer has requested changes to this PR awaiting reviews This PR is ready to be reviewed labels Nov 27, 2020
@amaank404
Copy link
Contributor

try this solution every time pre-commit fails:

  • pip install pre-commit
  • pre-commit run -a (It might show you it failed but it actually did not)
  • Commit the changes and push them into your fork

@arif599
Copy link
Contributor Author

arif599 commented Nov 28, 2020

try this solution every time pre-commit fails:

* `pip install pre-commit`

* `pre-commit run -a` (It might show you it failed but it actually did not)

* Commit the changes and push them into your fork

Thanks for letting me know! I have done what you suggested. I'll commit again.

@ghost ghost added awaiting reviews This PR is ready to be reviewed and removed tests are failing Do not merge until tests pass labels Nov 28, 2020
@dhruvmanila
Copy link
Member

pre-commit is a hook you install in your local git directory. Once it is installed, it will run automatically every time you commit your changes, but this is only for the local directory.

If you've downloaded pre-commit locally, then go to the root of your Python repository copy and run pre-commit install. That's it! Now, you don't have to worry about the pre-commit checks ever failing.

@dhruvmanila
Copy link
Member

@xcodz-dot If you give instructions to users about installing pre-commit, then please tell them to do pre-commit install instead of pre-commit run -a. This will save you and the user a ton of time and energy.

Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

LGTM!

@ghost ghost removed the awaiting reviews This PR is ready to be reviewed label Nov 28, 2020
@dhruvmanila dhruvmanila merged commit 9c6080a into TheAlgorithms:master Nov 28, 2020
@arif599
Copy link
Contributor Author

arif599 commented Nov 28, 2020

Seriously thank you guys for helping and guiding me. I'm a little new to this but I'll continue to learn and make more meaningful contributions along the way!

@amaank404
Copy link
Contributor

@xcodz-dot If you give instructions to users about installing pre-commit, then please tell them to do pre-commit install instead of pre-commit run -a. This will save you and the user a ton of time and energy.

I will sure do that, i am just going to add that to contributing.md and will make a PR quickly

stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
* Adding __str__() function

* Removing white space

* Update data_structures/linked_list/__init__.py

Co-authored-by: xcodz-dot <[email protected]>

* Adding type hints

* Update __init__.py

* Update __init__.py

* Adding the changes requested

* Updating to fix pre-commit

* Updating __init__.py

* Updating __init__.py

Co-authored-by: xcodz-dot <[email protected]>
peRFectBeliever pushed a commit to peRFectBeliever/Python that referenced this pull request Apr 1, 2021
* Adding __str__() function

* Removing white space

* Update data_structures/linked_list/__init__.py

Co-authored-by: xcodz-dot <[email protected]>

* Adding type hints

* Update __init__.py

* Update __init__.py

* Adding the changes requested

* Updating to fix pre-commit

* Updating __init__.py

* Updating __init__.py

Co-authored-by: xcodz-dot <[email protected]>
Panquesito7 pushed a commit to Panquesito7/Python that referenced this pull request May 13, 2021
* Adding __str__() function

* Removing white space

* Update data_structures/linked_list/__init__.py

Co-authored-by: xcodz-dot <[email protected]>

* Adding type hints

* Update __init__.py

* Update __init__.py

* Adding the changes requested

* Updating to fix pre-commit

* Updating __init__.py

* Updating __init__.py

Co-authored-by: xcodz-dot <[email protected]>
shermanhui pushed a commit to shermanhui/Python that referenced this pull request Oct 22, 2021
* Adding __str__() function

* Removing white space

* Update data_structures/linked_list/__init__.py

Co-authored-by: xcodz-dot <[email protected]>

* Adding type hints

* Update __init__.py

* Update __init__.py

* Adding the changes requested

* Updating to fix pre-commit

* Updating __init__.py

* Updating __init__.py

Co-authored-by: xcodz-dot <[email protected]>
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