Skip to content
This repository has been archived by the owner on Sep 25, 2022. It is now read-only.

Cameragroup #33

Merged
merged 10 commits into from
Jun 9, 2018
Merged

Cameragroup #33

merged 10 commits into from
Jun 9, 2018

Conversation

mohammadbashiri
Copy link
Member

Added the CameraGroup class, with some tests. One thing to note is the test for the look_at methods of the child cameras of a CameraGroup instance. it works only for a special case of point coordinate (x=0, y=0, z=-1).

@coveralls
Copy link

coveralls commented Jun 8, 2018

Pull Request Test Coverage Report for Build 125

  • 26 of 26 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+3.7%) to 79.827%

Totals Coverage Status
Change from base Build 116: 3.7%
Covered Lines: 1013
Relevant Lines: 1269

💛 - Coveralls

@nickdelgrosso
Copy link
Member

@mohammadbashiri The tests are failing for Python 2.7 and 3.4. Looks like it's because of the @ symbol used for the dot product. Could you pleace change this to the numpy.dot() function or method?

Copy link
Member

@nickdelgrosso nickdelgrosso left a comment

Choose a reason for hiding this comment

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

Files in the .egg directory shouldn't be added to the repo .
It looks like the .gitignore file missed it. Could you please remove these files from the pull request?

@mohammadbashiri
Copy link
Member Author

Sorry, i did not see the message about the .egg file. I will change that too.

@nickdelgrosso
Copy link
Member

The single-case test for CameraGroup.look_at() worries me. I've added a test for Physical.look_at() to provide extra security for this method. @mohammadbashiri could you review and merge this Pull request: mohammadbashiri#1 Then we'll have more stability

added test for Physical.look_at()
@nickdelgrosso
Copy link
Member

Great! Merging.

@nickdelgrosso nickdelgrosso merged commit 474941b into ratcave:dev Jun 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants