jupyter-widgets / ipywidgets

Interactive Widgets for the Jupyter Notebook
https://ipywidgets.readthedocs.io
BSD 3-Clause "New" or "Revised" License
3.11k stars 947 forks source link

Why are IntSlider and FloatSlider completely independent? #721

Open jdemeyer opened 7 years ago

jdemeyer commented 7 years ago

It seems that the IntSlider and FloatSlider widgets are implemented completely independently, even though they do essentially the same thing for a different type. I would have expected that both classes would be specializations of, say, NumericSlider or even that there would be just one class which could handle both int and float.

I am asking this question because maybe I want sliders for other kind of numbers (e.g. numpy or SageMath numbers or fractions).

CC @SylvainCorlay

jasongrout commented 7 years ago

It looks like at least the views are shared: https://github.com/ipython/ipywidgets/blob/master/jupyter-js-widgets/src/widget_float.ts#L69

I thought there was more code sharing than apparently there actually is. It sounds like a good idea to refactor common functionality to a base class.

jdemeyer commented 7 years ago

Part of the reason is surely because of traitlets. Traitlets doesn't support something like general "numbers", only Int and Float.

SylvainCorlay commented 7 years ago

Do you think that there is a need for a Number trait type?

jdemeyer commented 7 years ago

There should probably be something like a number meta-trait type, say Number() where the current Int() is essentially equivalent to Number(int) and the current float equivalent to Number(float).

But I don't know anything about traitlets to be honest...

SylvainCorlay commented 7 years ago

Wow, we haven't gotten into meta trait types yet (like c++ templates )! 😄

That would be cool though. I was more thinking of a trait like Any that validate anything that is an instance of numbers.Number.

jdemeyer commented 7 years ago

Wow, we haven't gotten into meta trait types yet (like c++ templates )! :smile:

Well, you have Union which is a kind of meta-trait type.

SylvainCorlay commented 7 years ago

@minrk what do you think of a Real trait type corresponding to numbers.Real? That would be a good match for javascript's Number.

SylvainCorlay commented 7 years ago

Well, you have Union which is a kind of meta-trait type.

Not quite, I meant parameterized trait types :) - although we have Instance, but it is an identity, not some computation based on types.

jdemeyer commented 7 years ago

@minrk what do you think of a Real trait type corresponding to numbers.Real. That would be a good match for javascript's Number.

That would be useful, but it would just be Number(numbers.Real) in my proposal.

You could consider just changing Float to accept any numbers.Real instance as value, but I don't know how much that would break stuff.

SylvainCorlay commented 7 years ago

You could consider just changing Float to accept any numbers.Real instance as value, but I don't know how much that would break stuff.

Changing Float to Number(float) would probably work...

minrk commented 7 years ago

A Number(SomeABC) trait sounds reasonable to me. This is mostly the same as what you already get with Instance(Type), but presumably Number would also get things like bounds.

SylvainCorlay commented 7 years ago

Yeah, I was thinking of matching the numbers hierarchy (Number encompasses Complex...)

jdemeyer commented 7 years ago

Yeah, I was thinking of matching the numbers hierarchy

I don't see how that would work, since those are abstract types. I don't see the point of making a difference between IntegralSlider() and RationalSlider() for example.

You need concrete types for this because you want to control the type of widget.value. That's why I would propose something like NumericSlider(t) taking a type t as argument. This type should satisfy issubclass(t, Real).

SylvainCorlay commented 7 years ago

You need concrete types for this because you want to control the type of widget.value. That's why I would propose something like NumericSlider(t) taking a type t as argument. This type should satisfy issubclass(t, Real).

That is exactly what I had in mind. Just that complex is not an option :)

jdemeyer commented 7 years ago

That is exactly what I had in mind. Just that complex is not an option :)

But that's not at all "matching the numbers hierarchy", so I'm confused now...

SylvainCorlay commented 7 years ago

But that's not at all "matching the numbers hierarchy", so I'm confused now...

I meant creating traittypes for the different levels of the numbers hierarchy.

jdemeyer commented 7 years ago

I meant creating traittypes for the different levels of the numbers hierarchy.

You mean concrete trait types like RealNumber() or you mean meta-trait types like RealNumber(float)?

Like I said before, I would really prefer to determine the concrete type of widget.value (like you currently do with IntSlider and FloatSlider), not just anything which is an instance of numbers.Real.

SylvainCorlay commented 7 years ago

You mean concrete trait types like RealNumber() or you mean meta-trait types like RealNumber(float)?

The former.

jdemeyer commented 7 years ago

The former.

But that wouldn't solve my problem...

SylvainCorlay commented 7 years ago

My proposal is

the latter would be a better match with the js frontend which does not have a notion of int.

jdemeyer commented 7 years ago

the latter would be a better match with the js frontend which does not have a notion of int.

I don't know the internal implementation, but why would the js frontend even need to know the value of the widget? I would prefer if the js widget would be as basic as possible: it should only know "this is a slider with 100 marks and the current value is mark 39".

jasongrout commented 7 years ago

I don't know the internal implementation, but why would the js frontend even need to know the value of the widget?

As an example, our jslink/jsdlink widgets let you hook widget values up on the browser side, so you'd need to know the value there.