Skip to content

Create weight_conversion.py #3964

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Nov 27, 2020
Merged

Create weight_conversion.py #3964

merged 14 commits into from
Nov 27, 2020

Conversation

AnubhavSolanki
Copy link
Contributor

Describe your change:

Added a Weight Conversion Algorithm.

  • 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
@AnubhavSolanki AnubhavSolanki mentioned this pull request Nov 26, 2020
Closed
@amaank404
Copy link
Contributor

amaank404 commented Nov 26, 2020 •
edited
Loading

Fix (this is the easiest method to comply with contirbuting.md requirements):

  • pip install pre-commit
  • pre-commit run -a

@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
@AnubhavSolanki
Copy link
Contributor Author

@xcodz-dot Thank you for your response so what should I do now as all checks have already passed.

@amaank404
Copy link
Contributor

@dhruvmanila check this out

@amaank404
Copy link
Contributor

@AnubhavSolanki, You can mention maintainers if you feel like your work is done

amaank404
amaank404 suggested changes Nov 26, 2020
View reviewed changes
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.

Naming Convention

amaank404
amaank404 approved these changes Nov 26, 2020
View reviewed changes
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.

Great

@AnubhavSolanki
Copy link
Contributor Author

so now can we merge it?

@amaank404
Copy link
Contributor

Yeah wait for @dhruvmanila, only members can merge (I am just a guy helping people)

@AnubhavSolanki
Copy link
Contributor Author

Yeah wait for @dhruvmanila, only members can merge (I am just a guy helping people)

Oh GOT IT... THANKS BTW

dhruvmanila
dhruvmanila requested changes Nov 26, 2020
View reviewed changes
@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
@amaank404
Copy link
Contributor

@AnubhavSolanki, you can directly use the commit button near suggestions if you do not like manual working

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

@dhruvmanila, check this if it meets your requirements

@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
@AnubhavSolanki
Copy link
Contributor Author

@dhruvmanila, check this if it's ok.

@ghost ghost added awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass and removed tests are failing Do not merge until tests pass awaiting reviews This PR is ready to be reviewed labels Nov 27, 2020
dhruvmanila
dhruvmanila requested changes Nov 27, 2020
View reviewed changes
@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 and removed awaiting changes A maintainer has requested changes to this PR labels Nov 27, 2020
dhruvmanila
dhruvmanila reviewed Nov 27, 2020
View reviewed changes
conversions/weight_conversion.py
Comment on lines +275 to +280
if to_type not in KILOGRAM_CHART or from_type not in WEIGHT_TYPE_CHART:
raise ValueError(
f"Invalid 'from_type' or 'to_type' value: {from_type!r}, {to_type!r}\n"
f"Supported values are: {', '.join(WEIGHT_TYPE_CHART)}"
)
return value * KILOGRAM_CHART[to_type] * WEIGHT_TYPE_CHART[from_type]
Copy link
Member

Choose a reason for hiding this comment

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

The reason for this change is so that the static type checker mypy does not complain that the function had no return statement.

If we did the try ... except ... pattern, when the try block raises an exception, we go to the except block where we would normally just print a message. This would mean that the function returns None implicitly, thus the mypy error.

Hope you understand the choice, if not please feel free to ask any question.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you got your point.. :)

dhruvmanila
dhruvmanila approved these changes Nov 27, 2020
View reviewed changes
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.

Awesome work! Thank you for your contribution.

@ghost ghost removed the awaiting reviews This PR is ready to be reviewed label Nov 27, 2020
@dhruvmanila dhruvmanila merged commit 9333d2f into TheAlgorithms:master Nov 27, 2020
stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
@AnubhavSolanki @dhruvmanila @stokhos
Create weight_conversion.py (TheAlgorithms#3964)
d410481 
* Create weight_conversion.py
* Update weight_conversion.py

Co-authored-by: Dhruv Manilawala <[email protected]>
peRFectBeliever pushed a commit to peRFectBeliever/Python that referenced this pull request Apr 1, 2021
@AnubhavSolanki @dhruvmanila @peRFectBeliever
Create weight_conversion.py (TheAlgorithms#3964)
b594928 
* Create weight_conversion.py
* Update weight_conversion.py

Co-authored-by: Dhruv Manilawala <[email protected]>
Panquesito7 pushed a commit to Panquesito7/Python that referenced this pull request May 13, 2021
@AnubhavSolanki @dhruvmanila
Create weight_conversion.py (TheAlgorithms#3964)
de3243b 
* Create weight_conversion.py
* Update weight_conversion.py

Co-authored-by: Dhruv Manilawala <[email protected]>
shermanhui pushed a commit to shermanhui/Python that referenced this pull request Oct 22, 2021
@AnubhavSolanki @dhruvmanila @shermanhui
Create weight_conversion.py (TheAlgorithms#3964)
5e400a1 
* Create weight_conversion.py
* Update weight_conversion.py

Co-authored-by: Dhruv Manilawala <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amaank404 amaank404 amaank404 approved these changes

@dhruvmanila dhruvmanila dhruvmanila approved these changes

@BryanLaura BryanLaura BryanLaura approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AnubhavSolanki @amaank404 @dhruvmanila @BryanLaura