Added Mirror Formulae Equation #9717

vipinkarthic · 2023-10-04T16:41:39Z

Describe your change:

This PR adds an algorithm involving the Mirror formulae for finding, the Focal Length, Object Distance from mirror, Image Distance form the mirror.

Add an algorithm?
Fix a bug or typo in an existing algorithm?
Documentation change

Checklist:

cclauss · 2023-10-04T16:52:06Z

`python3 -m doctest -v physics/mirror_formulae.py`

=================================== FAILURES ===================================
________________ [doctest] physics.mirror_formulae.focal_length ________________
062
063 >>> focal_length(10, 20)
Expected:
6.666666666666667
Got:
6.666666666666666

/home/runner/work/Python/Python/physics/mirror_formulae.py:63: DocTestFailure
_______________ [doctest] physics.mirror_formulae.image_distance _______________
104
105 >>> image_distance(10, 40)
Expected:
13.33333333
Got:
8.0

/home/runner/work/Python/Python/physics/mirror_formulae.py:105: DocTestFailure
______________ [doctest] physics.mirror_formulae.object_distance _______________
083
084 >>> object_distance(30, 20)
Expected:
-60
Got:
-59.999999999999986

/home/runner/work/Python/Python/physics/mirror_formulae.py:84: DocTestFailure
=============================== warnings summary ===============================

cclauss · 2023-10-04T16:54:02Z

You might need to use https://docs.python.org/3/library/math.html#math.isclose to make your doctests work on all platforms.

vipinkarthic · 2023-10-04T20:05:58Z

You might need to use https://docs.python.org/3/library/math.html#math.isclose to make your doctests work on all platforms.

Hello !

Thanks for looking into my PR, but i am really confused on how to implement the math.isclose() function in this case since it returns a boolean value. I would be very grateful if you can help me with how to implement it.

In the meantime I did try using the round() function, since it gives a more simplified answer, would love to know if that's ok or I should change it to the math.isclose() function.

cclauss · 2023-10-04T21:19:06Z

>>> from math import is_close
>>> is_close(focal_length(10, 20), 6.66666666666666)
True

for more information, see https://pre-commit.ci

vipinkarthic · 2023-10-05T02:46:19Z

I will recheck where I'm going wrong and commit the changes soon.

algorithms-keeper

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

Contributing guidelines

Project Euler solution guidelines

Python:

Formatted string literals (f-strings)

Type hints

doctest

unittest

pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

@algorithms-keeper review to trigger the checks for only added pull request files

@algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

algorithms-keeper · 2023-10-05T03:45:59Z

physics/mirror_formulae.py

+"""
+
+<<<<<<< HEAD
+=======


An error occurred while parsing the file: physics/mirror_formulae.py

Traceback (most recent call last): File "/opt/render/project/src/algorithms_keeper/parser/python_parser.py", line 146, in parse reports = lint_file( ^^^^^^^^^^ libcst._exceptions.ParserSyntaxError: Syntax Error @ 61:3. parser error: error at 60:2: expected one of (, *, +, -, ..., AWAIT, EOF, False, NAME, NUMBER, None, True, [, break, continue, lambda, match, not, pass, ~ ======= ^

vipinkarthic · 2023-10-05T03:53:22Z

I have resolved the issue, Please let me know if any more changes are to be made ! 😄

cclauss · 2023-10-05T04:52:13Z

physics/mirror_formulae.py

@@ -57,13 +57,13 @@

 """

-
+from math import isclose


Please move this import into the doctest so that the linter does not get confused.

cclauss · 2023-10-05T04:53:32Z

physics/mirror_formulae.py

+    >>> isclose(object_distance(30, 20), -60.0)
+    True
+    >>> isclose(object_distance(10.5, 11.7), 102.375)
+    True


~~I do not think we need is_close() here.~~ Let’s only use it where we need to.

When running the doctests locally, there were issues when used without the isclose, should i still remove them or leave it as it is ?

Ok. We need is_close()

Is there anything else I should add/change ?, or just have to wait till PR is merged ?

cclauss · 2023-10-05T05:58:12Z

Nice one!!

vipinkarthic · 2023-10-05T06:03:02Z

Thank You, I'm looking forward add more 😄

cclauss · 2023-10-05T06:19:36Z

Problems...

https://github.com/TheAlgorithms/Python/actions/runs/6415330109/job/17417072625?pr=9778#step:6:716
Scroll to line 716.

cclauss · 2023-10-05T06:23:51Z

physics/mirror_formulae.py

+    >>> from math import isclose
+    >>> isclose(focal_length(9.5, 6.7), 3.929012346)
+    True
+    >>> focal_length(0, 20)


>>> focal_length(0, 20) # doctest: +NORMALIZE_WHITESPACE

cclauss · 2023-10-05T06:24:14Z

physics/mirror_formulae.py

+    >>> from math import isclose
+    >>> isclose(object_distance(10.5, 11.7), 102.375)
+    True
+    >>> object_distance(90, 0)


# doctest: +NORMALIZE_WHITESPACE

cclauss · 2023-10-05T06:24:44Z

physics/mirror_formulae.py

+    >>> from math import isclose
+    >>> isclose(image_distance(1.5, 6.7), 1.932692308)
+    True
+    >>> image_distance(0, 0)


# doctest: +NORMALIZE_WHITESPACE

cclauss · 2023-10-05T06:26:06Z

Can you please make a new PR with those three comments added so the build tests pass?

vipinkarthic · 2023-10-05T07:19:24Z

Yes, I will do it right now

vipinkarthic · 2023-10-05T07:28:16Z

I have created a added ( Normalize Whitespace ) and created the Pull request.
https://github.com/TheAlgorithms/Python/pull/9782

* Python mirror_formulae.py is added to the repository * Changes done after reading readme.md * Changes for running doctest on all platforms * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Change 2 for Doctests * Changes for doctest 2 * updating DIRECTORY.md --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: github-actions <${GITHUB_ACTOR}@users.noreply.github.com>

vipinkarthic and others added 4 commits October 4, 2023 20:47

Python mirror_formulae.py is added to the repository

3abb0b4

Changes done after reading readme.md

bc1942a

Merge branch 'TheAlgorithms:master' into master

acc30f0

Merge branch 'TheAlgorithms:master' into master

def94b6

algorithms-keeper bot added awaiting reviews This PR is ready to be reviewed tests are failing Do not merge until tests pass labels Oct 4, 2023

vipinkarthic and others added 3 commits October 5, 2023 08:00

Changes for running doctest on all platforms

d0979d3

Merge branch 'master' of https://github.com/vipinkarthic/Python

22f8647

[pre-commit.ci] auto fixes from pre-commit.com hooks

e5f2d76

for more information, see https://pre-commit.ci

vipinkarthic and others added 4 commits October 5, 2023 09:12

Change 2 for Doctests

0ed896b

Merge branch 'TheAlgorithms:master' into master

987068a

Merge branch 'master' of https://github.com/vipinkarthic/Python

87cc27c

Merge branch 'master' of https://github.com/vipinkarthic/Python

94b630c

algorithms-keeper bot reviewed Oct 5, 2023

View reviewed changes

vipinkarthic and others added 2 commits October 5, 2023 09:19

Changes for doctest 2

8affc05

updating DIRECTORY.md

e250ede

cclauss reviewed Oct 5, 2023

View reviewed changes

cclauss approved these changes Oct 5, 2023

View reviewed changes

algorithms-keeper bot removed the awaiting reviews This PR is ready to be reviewed label Oct 5, 2023

cclauss merged commit 935d1d3 into TheAlgorithms:master Oct 5, 2023

cclauss reviewed Oct 5, 2023

View reviewed changes

isidroas mentioned this pull request Jan 25, 2025

dh isidroas/Python#1

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Mirror Formulae Equation #9717

Added Mirror Formulae Equation #9717

vipinkarthic commented Oct 4, 2023

cclauss commented Oct 4, 2023

cclauss commented Oct 4, 2023

vipinkarthic commented Oct 4, 2023

cclauss commented Oct 4, 2023

vipinkarthic commented Oct 5, 2023

algorithms-keeper bot left a comment

algorithms-keeper bot Oct 5, 2023

vipinkarthic commented Oct 5, 2023 •

edited

Loading

cclauss Oct 5, 2023

cclauss Oct 5, 2023 •

edited

Loading

vipinkarthic Oct 5, 2023

cclauss Oct 5, 2023

vipinkarthic Oct 5, 2023 •

edited

Loading

cclauss commented Oct 5, 2023

vipinkarthic commented Oct 5, 2023

cclauss commented Oct 5, 2023

cclauss Oct 5, 2023

cclauss Oct 5, 2023 •

edited

Loading

cclauss Oct 5, 2023

cclauss commented Oct 5, 2023

vipinkarthic commented Oct 5, 2023

vipinkarthic commented Oct 5, 2023

Added Mirror Formulae Equation #9717

Added Mirror Formulae Equation #9717

Conversation

vipinkarthic commented Oct 4, 2023

Describe your change:

Checklist:

cclauss commented Oct 4, 2023

python3 -m doctest -v physics/mirror_formulae.py

cclauss commented Oct 4, 2023

vipinkarthic commented Oct 4, 2023

cclauss commented Oct 4, 2023

vipinkarthic commented Oct 5, 2023

algorithms-keeper bot left a comment

Choose a reason for hiding this comment

🔗 Relevant Links

Repository:

Python:

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

algorithms-keeper actions can be triggered by commenting on this PR:

algorithms-keeper bot Oct 5, 2023

Choose a reason for hiding this comment

vipinkarthic commented Oct 5, 2023 • edited Loading

cclauss Oct 5, 2023

Choose a reason for hiding this comment

cclauss Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

vipinkarthic Oct 5, 2023

Choose a reason for hiding this comment

cclauss Oct 5, 2023

Choose a reason for hiding this comment

vipinkarthic Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

cclauss commented Oct 5, 2023

vipinkarthic commented Oct 5, 2023

cclauss commented Oct 5, 2023

cclauss Oct 5, 2023

Choose a reason for hiding this comment

cclauss Oct 5, 2023 • edited Loading

Choose a reason for hiding this comment

cclauss Oct 5, 2023

Choose a reason for hiding this comment

cclauss commented Oct 5, 2023

vipinkarthic commented Oct 5, 2023

vipinkarthic commented Oct 5, 2023

`python3 -m doctest -v physics/mirror_formulae.py`

vipinkarthic commented Oct 5, 2023 •

edited

Loading

cclauss Oct 5, 2023 •

edited

Loading

vipinkarthic Oct 5, 2023 •

edited

Loading

cclauss Oct 5, 2023 •

edited

Loading