Agreed about the factor range class check though. 



> On Jun 3, 2014, at 7:26, Mateusz Paprocki <notifications@github.com> wrote:
> 
> In bokehjs/src/coffee/common/plot.coffee:
> 
> > @@ -123,29 +122,55 @@ define [
> >  
> >        @hidpi = options.hidpi ? @mget('hidpi')
> >  
> > -      @x_range = options.x_range ? @mget_obj('x_range')
> > -      @y_range = options.y_range ? @mget_obj('y_range')
> > -
> > -      xmapper_type = LinearMapper.Model
> > -      if @x_range.type == "FactorRange"
> > -        xmapper_type = CategoricalMapper.Model
> > -      @xmapper = new xmapper_type({
> > -        source_range: @x_range
> > +      @x_ranges = {}
> > +      @x_mappers = {}
> > +      x_range = options.x_range ? @mget_obj('x_range')
> > +      x_mapper_type = LinearMapper.Model
> > +      if x_range.type == "FactorRange"
> I know this is WIP, but this code just asks for being source of bugs in future. Given that x_range is an object (thanks to @mget_obj()), then this should use subclass relation instead. Otherwise, what will happen when a subclass of FactorRange is introduced? If we desire to definitively not subclass FactorRange, then it should be marked as final to denote intended behavior (Python doesn't support final, override, private, etc. modifiers, but we can at least comment on that, or use a new decorator for that purpose). But even then I would prefer using proper techniques. Furthermore, this code is repeated several times, so maybe each Range's subtype should provide correct implementation of its mapper. Then this code would boil down to, e.g. x_range.mapper_type. Unfortunately, FactorRange inherits directly from HasProperties (the s ame for Range1d) and there is no Range abstract class. There is also a subclass of FactorRange: DataFactorRange.
> 
> —
> Reply to this email directly or view it on GitHub.