Skip to content

re-export PolarLineRenderer and PolarMapper from chaco.api#732

Merged
aaronayres35 merged 2 commits into
masterfrom
reexport-polar-classes
May 10, 2021
Merged

re-export PolarLineRenderer and PolarMapper from chaco.api#732
aaronayres35 merged 2 commits into
masterfrom
reexport-polar-classes

Conversation

@aaronayres35
Copy link
Copy Markdown
Contributor

@aaronayres35 aaronayres35 commented May 7, 2021

closes #629

This PR simply re-exports the 2 classes from the api module.

It also pulls a comment about PolarMapper currently just being LinearMapper into the docstring so it appears on api docs. Eventually we will want to update it, see #47. In the meanwhile it is fine to like in the api as long as this is documented.
(Actually looking at it, it looks like some changes have been made to LinearMapper that weren't copied over... I may go ahead and pull those directly so that the PolarMapper really does just match the LinearMapper like it says).
EDIT: done. The PolarMapper is now the current LinearMapper rather than an old version of the LinearMapper.

@aaronayres35 aaronayres35 requested a review from rahulporuri May 7, 2021 19:54
Comment thread chaco/polar_mapper.py
Comment thread chaco/polar_mapper.py
Comment thread chaco/polar_mapper.py
Comment thread chaco/polar_mapper.py
Comment on lines +50 to +53
if isinstance(data_array, (tuple, list, ndarray)):
return full_like(data_array, self.low_pos, dtype=float64)
else:
return array([self.low_pos])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure why these changes were made. Were these changes copied over from the linear mapper?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have the same comment about the changes in map_data and _compute_scale

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Were these changes copied over from the linear mapper?

Yeah, I effectively copied the current LinearMapper here

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

LGTM. I haven't tested to check that the example still works.

@aaronayres35 aaronayres35 merged commit 59a3428 into master May 10, 2021
@aaronayres35 aaronayres35 deleted the reexport-polar-classes branch May 10, 2021 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Re-export PolarLineRenderer and PolarMapper from chaco.api

2 participants