Skip to content

add scatter_or_line argument to Learner1D.plot#215

Merged
jbweston merged 3 commits intomasterfrom
learner1D_plot
Oct 14, 2019
Merged

add scatter_or_line argument to Learner1D.plot#215
jbweston merged 3 commits intomasterfrom
learner1D_plot

Conversation

@basnijholt
Copy link
Member

I've needed this many times and the discussion in the chat now also made it clear that this is needed.

Now you won't have to look at plots like this:
image

@akhmerov
Copy link
Contributor

akhmerov commented Sep 5, 2019

Not sure; we're already using not-a-plotting-library (holoviews), so I'd rather keep adaptive's plotting interface as small as possible.

Copy link
Contributor

@jbweston jbweston left a comment

Choose a reason for hiding this comment

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

I disagree with anton here. BaseLearner has no plotting method, so any additions we make to the plotting APIs only affect individual learners, and here I think we should make the interface as user-friendly as possible

Copy link
Contributor

@jbweston jbweston left a comment

Choose a reason for hiding this comment

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

LGTM

@jbweston jbweston merged commit 187f88f into master Oct 14, 2019
@basnijholt basnijholt deleted the learner1D_plot branch October 17, 2019 10:12
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.

3 participants