Skip to content

Tortilla #7201

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

Closed
wants to merge 7 commits into from
Closed

Tortilla #7201

wants to merge 7 commits into from

Conversation

DIvkov575
Copy link
Contributor

Describe your change:

  • Add an algorithm?

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}.

@algorithms-keeper algorithms-keeper bot added the awaiting reviews This PR is ready to be reviewed label Oct 15, 2022
@algorithms-keeper algorithms-keeper bot added the tests are failing Do not merge until tests pass label Oct 15, 2022
V = sqrt(2 * gravity * Δy)

Source:
- https://en.wikipedia.org/wiki/Archimedes%27_principle
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- https://en.wikipedia.org/wiki/Archimedes%27_principle
- https://en.wikipedia.org/wiki/Torricelli%27s_law

Calculates velocity of fluid from Potential Gravitational Energy

Equation for finding velocity
V = sqrt(2 * gravity * Δy)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
V = sqrt(2 * gravity * Δy)
Velocity = sqrt(2 * Acceleration due to gravity * Height)

Args:
height: The change in height between initial and point of flow (where,
velocity is being measured)
gravity: Acceleration from gravity. Gravitational force on system,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
gravity: Acceleration from gravity. Gravitational force on system,
gravity: Acceleration due to gravity. Gravitational force on system,

"""
Args:
height: The change in height between initial and point of flow (where,
velocity is being measured)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please mention the unit of height used.

7.6681158050723255
"""
if gravity <= 0:
raise ValueError("Impossible Gravity")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise ValueError("Impossible Gravity")
raise ValueError("The value of g cannot be equal or smaller than 0.")

returns:
buoyant force on object in Newtons

>>> archimedes_principle(fluid_density=997, volume=0.5, gravity=9.8)
Copy link
Contributor

Choose a reason for hiding this comment

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

If the default value of g is 9.8, you can add a new test case for value of g different than 9.8; otherwise, it seems redundant.

"""

if fluid_density <= 0:
raise ValueError("Impossible fluid density")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise ValueError("Impossible fluid density")
raise ValueError("Fluid density cannot be equal or smaller than 0.")

Comment on lines +38 to +40
raise ValueError("Impossible Object volume")
if gravity <= 0:
raise ValueError("Impossible Gravity")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise ValueError("Impossible Object volume")
if gravity <= 0:
raise ValueError("Impossible Gravity")
raise ValueError("Object volume cannot smaller than 0")
if gravity <= 0:
raise ValueError("Acceleration due to gravity cannot be equal or smaller than 0.")

@ZeroDayOwl
Copy link
Contributor

@DIvkov575 make one PR per algorithm. You have duplications present in #7143 and #7200

@cclauss
Copy link
Member

cclauss commented Oct 16, 2022

Hacktoberfest is strenuous for our team of active volunteer maintainers. That small team now has 200+ open pull requests to review. We have high standards for our submissions which are described in CONTRIBUTING.md. Our continuous integration tests help to ensure that our standards are met before we review and merge deserving pull requests. This is further complicated by the fact that we already have lots of algorithms and would like to avoid duplicates.

For these reasons, I will close all open pull requests failing tests so that our unpaid team has a smaller mountain to climb. If you want us to land your pull request, please read CONTRIBUTING.md carefully and try again.

@cclauss cclauss closed this Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants