A few changes

[giovannipizzi]

Here a few minor changes that I would do:

• rename in init noselect_color -> unselected_color to be consistent with selected_colors
• In this line:

  widget-periodictable/widget_periodictable/periodic_table.py

  Line 41 in 86a8e17

  if len(selected_colors) < states:

  you might want to actually add that list enough times if state is very large. Something like

   additional_colors = ["#a6cee3", "#b2df8a", "#fdbf6f", "#6a3d9a", "#b15928", "#e31a1c", "#1f78b4", "#33a02c", "#ff7f00", "#cab2d6", "#ffff99"] 
   self.selected_colors = selected_colors + additional_colors * (1 + (states - len(selected_colors)) // len(additional_colors)))

  (I didn't test it, maybe double check)
• Typo here:

  widget-periodictable/src/widget.ts

  Line 93 in 86a8e17

  'else if (disabledElements.includes(elementName)){print("disabled");} else {print("noselcted");} %>" ><% '+

  
  (replace noselcted with unselected)
• If you want to show a tooltip, then it might be good to ask the user to give names to each state. I think in the application the user wants to read "excluded" rather than "state: 1". I think the easiest for now is to disable the tooltip (not really needed), and in a later version we can ask (optionally) names for each state as a list of strings in the init
• Make sure binder starts in app mode (it didn't for me)
• there is a typo in the jupyter notebook used in binder: on_set_from_ptyhon
• selecting from python seems buggy:
  • in the example, if you call widget.selected_elements = ['Li', 'H'], they get selected but with different state. How do I chose the state?
  • if call the python function when other things are selected, the other elements seem to get deselected, but then if I click on e.g. Li to change its state, they jump back (at least it happened to me once, I cannot reproduce). Definitely one thing I can reproduce is that if I select with different states some other elements, say 'Si' and 'Ge', and then I call widget.selected_elements = ['Li', 'H'], when I click on Si and Ge (now unselected) they don't start from state 0, but from some other state.
  • If I have things in various states, e.g. widget.selected_elements is ['Li', 'H', 'B', 'Al', 'Ga', 'In'] and widget.selected_states is [0, 1, 2, 0, 0, 0, 0], a call to widget.get_elements_by_state(0) crashes with

    ---------------------------------------------------------------------------
    IndexError                                Traceback (most recent call last)
    <ipython-input-32-b8b33d072fad> in <module>
    ----> 1 widget.get_elements_by_state(0)
    
    /srv/conda/envs/notebook/lib/python3.7/site-packages/widget_periodictable/periodic_table.py in get_elements_by_state(self, state)
         46         for i, j in enumerate(self.selected_states):
         47             if j == state:
    ---> 48                 x.append(self.selected_elements[i])
         49         return x
    
    IndexError: list index out of range
    

    I think you can replace the function with just this one-liner?

    return [element for element, element_state in zip(widget.selected_elements, widget.selected_states) if element_state==state]

[giovannipizzi]

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

[dou-du]

@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]

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]

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.