Skip to content
This repository was archived by the owner on Jul 26, 2020. It is now read-only.

Conversation

trickeydan
Copy link

Do not merge until this system has been removed from both docs and robot-api

@trickeydan trickeydan requested review from PeterJCLaw and Adimote June 24, 2018 17:10
@trickeydan
Copy link
Author

trickeydan commented Jun 24, 2018

@Adimote There's a strange failure with the tests here. The linting complains that np is unused in 38baa8d and 741d705, yet when I removed it in a89f0bf a load of errors occur due to np not being imported.

Copy link
Contributor

@PeterJCLaw PeterJCLaw left a comment

Choose a reason for hiding this comment

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

Seems good; I'll have a more complete look when I've time to test it out.

We should check that robotd (and robot-api) have stopped using these features before removing them. I know we want to do that, so hopefully this is just a case of remove the dead code there too (first).

CircleCI is highlighting that there's an import of numpy in coordinates.py which is unused. Note that coordinates.py imports numpy both as a module and some functions from it; only one of these can be removed.
(It looks like the CI previously correctly highlighted the erroneous removal of a similar import from tokens.py)


# Polar co-ordinates in the 3D world, relative to the camera
self.polar = cartesian_to_legacy_polar(self.cartesian)
self.legacy_polar = cartesian_to_legacy_polar(self.cartesian)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should ensure that the robotd changes to remove legacy_polar happen before this change is merged.

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.

2 participants