osscar-org / widget-periodictable

A jupyter widget to select chemical elements from the periodic table.
Other
9 stars 3 forks source link

A few changes #4

Open giovannipizzi opened 4 years ago

giovannipizzi commented 4 years ago

Here a few minor changes that I would do:

giovannipizzi commented 4 years ago

@dou-du I suggest that we start working with PRs so I can give feedback before it's merged?

dou-du commented 4 years ago

@dou-du I suggest that we start working with PRs so I can give feedback before it's merged?

Sure, I will work on the develop branch.

dou-du commented 4 years ago

Hi Giovanni @giovannipizzi ,

This issue is quite problematic to fix. First, I was struggling with a Traitlets bug:

For example, self.selected_elements[1] = "H" will not trigger the Javascript function. This is a traitlets bug. Other people also notice this.

For the selected_states and selected_elements, I try to consider all the possible cases: if we give more states than elements, it will truncate the same length of the selected elements. When we init the elements through w.selected_elements: 1 if the element had been selected before, it will keep intact.
2 if the element is not selected before, it will be set as state zero.

I also update the binder docs. Please check my pull request #13

dou-du commented 4 years ago

There are more bugs. Forget about the pull request. I think it is very problematic to have two separate variable (selected_elements, selected_states). I will use a dictionary instead.