Callback triggering broken after change from String to Array in FileInput widget

Hello,
I’m currently trying to amend the FileInput widget with support for
multiple file selection.
This is prevented in the original (branch “master”) by a hardcoded
this.dialogEl.multiple = false
in bokeh/bokehjs/src/lib/models/widgets/file_input.ts.

I set-up a dedicated virtual machine with ubuntu 18.04, conda with python 3.8 according to instruction from https://docs.bokeh.org/en/latest/docs/dev_guide/setup.html#devguide-setup

The Problem:
generally speaking I modified
- bokeh/bokehjs/src/lib/models/widgets/file_input.ts
- bokeh/bokeh/models/widgets/inputs.py
to use Array/List for the properties value, mimetype, filename.
And wrote a small test case for bokeh server to check.
(The code is below)

And I can see in the browser-console that the JS-part is working as expected.
But there is no longer any callback triggered on the Python-part neither
FileInput.on_change() nor curdoc().on_change() is triggered.

I crosschecked the use of Array/List with other bokeh components (bokehjs/src/lib/models/widgets/checkbox_button_group.ts), but could not detect the error in my coding.

Can someone from the experts have a look, please?

Sources:

file_input.ts

import * as p from "core/properties"
import {Widget, WidgetView} from "models/widgets/widget"

export class FileInputView extends WidgetView {
  model: FileInput

  protected dialogEl: HTMLInputElement

  connect_signals(): void {
    super.connect_signals()
    this.connect(this.model.change, () => this.render())
    this.connect(this.model.properties.width.change, () => this.render())
  }

  render(): void {
    if (this.dialogEl == null) {
      this.dialogEl = document.createElement('input')
      this.dialogEl.type = "file"
      console.log("DEBUG: file_input.ts:")
      this.dialogEl.multiple = false  
      this.dialogEl.onchange = (e) => this.load_file(e)
      this.el.appendChild(this.dialogEl)
    }
    if (this.model.accept != null && this.model.accept != '')
      this.dialogEl.accept = this.model.accept
    if (this.model.multiple == 'multiple' || this.model.multiple == 'True'){
      this.dialogEl.multiple = true
      this.dialogEl.onchange = (e) => this.load_files(e)
    }
    this.dialogEl.style.width = `{this.model.width}px`
    this.dialogEl.disabled = this.model.disabled
  }

  load_file(e: any): void {
    const reader = new FileReader()
    this.model.filename.push(e.target.files[0].name)
    reader.onload = (e) => this.file(e)
    reader.readAsDataURL(e.target.files[0])
  }

  load_files(e: any): void {
    const files = e.target.files
    var i : number
    for (i=0; i< files.length ; i++){
        //console.log(files[i].name);
        this.readfile(files[i])
    }
    console.log(this.model.filename)
  }
  
  readfile(file: any): void {
    const reader = new FileReader()
    this.model.filename.push(file.name)
    reader.onload = (e) => this.file(e)
    reader.readAsDataURL(file)
  }

  file(e: any): void {
    const file = e.target.result
    const file_arr = file.split(",")

    const content = file_arr[1]
    const header = file_arr[0].split(":")[1].split(";")[0]
    
    this.model.value.push(content)
    this.model.mime_type.push(header)
    //console.log(content)
  }
}

export namespace FileInput {
  export type Attrs = p.AttrsOf<Props>
  export type Props = Widget.Props & {
    value: p.Property<string[]>
    mime_type: p.Property<string[]>
    filename: p.Property<string[]>
    accept: p.Property<string>
    multiple: p.Property<string>
  }
}

export interface FileInput extends FileInput.Attrs {}

export abstract class FileInput extends Widget {

  properties: FileInput.Props

  constructor(attrs?: Partial<FileInput.Attrs>) {
    super(attrs)
  }

  static init_FileInput(): void {
    this.prototype.default_view = FileInputView

    this.define<FileInput.Props>({
      value:     [ p.Array, [] ],
      mime_type: [ p.Array, [] ],
      filename:  [ p.Array, [] ],
      accept:    [ p.String, '' ],
      multiple:  [ p.String, '' ],
    })
  }
}

inputs.py (only FileInput API)

class FileInput(Widget):
    ''' Present a file-chooser dialog to users and return the contents of a
    selected file.

    '''
    
    value = List(String(  help="""
    A base64-encoded string of the contents of the selected file.
    """), default=[])

    mime_type = List(String( default="", readonly=True, help="""
    The mime type of the selected file.
    """))

    filename = List(String( help="""
    The filename of the selected file.
    .. note::
        The full file path is not included since browsers will not provide
        access to that information for security reasons.
    """),default=[])

    accept = String(default="", help="""
    Comma-separated list of standard HTML file input filters that restrict what
    files the user can pick from. Values can be:

    `<file extension>`:
        Specific file extension(s) (e.g: .gif, .jpg, .png, .doc) are pickable

    `audio/*`:
        all sound files are pickable

    `video/*`:
        all video files are pickable

    `image/*`:
        all image files are pickable

    `<media type>`:
        A valid `IANA Media Type`_, with no parameters.

    .. _IANA Media Type: https://www.iana.org/assignments/media-types/media-types.xhtml
    """)
        
    multiple = String(default="False", help="""
    set to "multiple" or "True" if selection of more than one file at a time
    should be supported.
    """)

(Testcase) test1.py:

from bokeh.io import curdoc
from bokeh.models.widgets import FileInput
from bokeh.layouts import column

#output_file("file_input.html")
def showfiles(attr,old, new):
    print("showfiles()")
    print(new)
    print(attr)
    print(len(new))
    
def docchange(event):
    print("document changed")
    print(event)

#file_input = FileInput() # This is for original bokeh 1.4
file_input = FileInput(multiple='True') # this is for the modified FileInput widget
file_input.on_change('filename', showfiles)

curdoc().add_root(column(file_input))
curdoc().on_change(docchange)

@ThomasB Bokeh properties are only able to automatically trigger update events when they are assigned to. It’ s not possible to automatically respond to changes made “in-place”. So my first speculation is that is to do with how you are using push to add new results to the existing list without ever assigning to the value property. You should re-arrange your code to collect the files, and then as the last action, do something like:

this.model.value = new_value

Seems like you would want to do that anyway, to reset the value for each batch of inputs.

Note the we will have to preserve the existing API, so that means either:

  • value becomes a union type, so that it’s a string when mulitple is False, and a list when it is true, or
  • make a separate MultiFileInput model

Thanks for the input.
Is there a preference from your side concerning the two options/bullet points?
While I reiterate over the code, anyway, I could try to comply with either.

@ThomasB I don’t have a strong preference, they both have their downsides :slight_smile: I would only note that in the “union” case we would need to default multiple to False, to preserve backwards compatibility. Otherwise I am happy to defer to your judgment.

@Bryan Further contemplation and looking at my TS-competence and my understanding of bokeh internals I would actually refrain from a new mixed data type (union)…
What about a “by-pass” like sticking with the current API and returning a JSON-string of the Array in case of multiple=‘True’ ?
In that case only the documentation has to be amended for the new case.
And users of the new case just have to json.loads(value) to get what they want.

I’d be -1 on that because json.load would be needed in some cases but not others and that is difficult to document and demonstrate in simple manner. It’s one thing to have different types that are clearly different, it’s much more confusing to have the same type mean mean different things at different times.

FWIW on the Python side it would be Either(String, List[String]), ...) and on the TS side it would be p.Property<string | string[]> in the namespace and just p.Any in the class.

Alternatively, MultiFileInput also seems fine.

With the “FWIW” info above, a colleague who knows “Promise”, “async” and “await” and some frustration resilience I was able to put code together, that works in my environment as an improved FileInput-widget.

I would be glad to see some independent test results. The sources …

file_input.ts:

import * as p from "core/properties"
import {Widget, WidgetView} from "models/widgets/widget"

export class FileInputView extends WidgetView {
  model: FileInput
  
  protected dialogEl: HTMLInputElement
  mime_type: string[] = []
  filename: string[] = []
  value: string[] = []

  connect_signals(): void {
    super.connect_signals()
    this.connect(this.model.change, () => this.render())
    this.connect(this.model.properties.width.change, () => this.render())
  }

  render(): void {
    if (this.dialogEl == null) {
      this.dialogEl = document.createElement('input')
      this.dialogEl.type = "file"
      console.log("DEBUG: file_input.ts:")
      this.dialogEl.multiple = false  
      this.dialogEl.onchange = async (e) => await this.load_files(e)
      this.el.appendChild(this.dialogEl)
    }
    if (this.model.accept != null && this.model.accept != '')
      this.dialogEl.accept = this.model.accept
    if (this.model.multiple == 'multiple' || this.model.multiple == 'True'){
      this.dialogEl.multiple = true
    }
    this.dialogEl.style.width = `{this.model.width}px`
    this.dialogEl.disabled = this.model.disabled
  }


  // multiple = True
  async load_files(e: any): Promise<void> {
    const files = e.target.files
    var i : number
    var value : string[] = []
    var filename : string[] = []
    var mime_type : string[] = []
    
    for (i=0; i< files.length ; i++){
      filename.push(files[i].name)
      let content = await this.readfile(files[i])
      const file_arr = content.split(",")
      value.push(file_arr[1])
      mime_type.push(file_arr[0].split(":")[1].split(";")[0])
      //console.log(content);
    }
    if (this.model.multiple == 'multiple' || this.model.multiple == 'True'){
      this.model.filename = filename
      this.model.mime_type= mime_type
      this.model.value = value
    }
    else {
      this.model.filename = filename[0]
      this.model.mime_type= mime_type[0]
      this.model.value = value[0]
    }
    console.log('load_files:')
    console.log(this.model.filename)
    console.log(this.model.value)
  }
  
  readfile(file: any): Promise<string> { //<string | ArrayBuffer | null>
    return new Promise<any>((resolve) => {
      const reader = new FileReader()
      reader.onload = (e) => resolve(e.target ? e.target.result : "")
      reader.readAsDataURL(file)
    })
  }
}

export namespace FileInput {
  export type Attrs = p.AttrsOf<Props>
  export type Props = Widget.Props & {
    value: p.Property<string|string[]>
    mime_type: p.Property<string|string[]>
    filename: p.Property<string|string[]>
    accept: p.Property<string>
    multiple: p.Property<string>
  }
}

export interface FileInput extends FileInput.Attrs {}

export abstract class FileInput extends Widget {

  properties: FileInput.Props

  constructor(attrs?: Partial<FileInput.Attrs>) {
    super(attrs)
  }

  static init_FileInput(): void {
    this.prototype.default_view = FileInputView

    this.define<FileInput.Props>({
      value:     [ p.Any, [] ],
      mime_type: [ p.Any, [] ],
      filename:  [ p.Any, [] ],
      accept:    [ p.String, '' ],
      multiple:  [ p.String, '' ],
    })
  }
}

inputs.py (only FileInput API part) :

class FileInput(Widget):
    ''' Present a file-chooser dialog to users and return the contents of a
    selected file.

    '''
    
    value = Either(String(default='', readonly=True,  help="""
    A base64-encoded string of the contents of the selected file.
    """),
                    List(String(default='', readonly=True,  help="""
    A base64-encoded string of the contents of the selected file.
    """)))

    mime_type = Either(String( default="", readonly=True, help="""
    The mime type of the selected file.
    """),
                        List(String( default="", readonly=True, help="""
    The mime type of the selected file.
    """)))

    filename = Either(String( default='', readonly=True, help="""
    The filename of the selected file.
    .. note::
        The full file path is not included since browsers will not provide
        access to that information for security reasons.
    """),
    List(String( default='', readonly=True, help="""
    The filename of the selected file.
    .. note::
        The full file path is not included since browsers will not provide
        access to that information for security reasons.
    """)))

    accept = String(default="", help="""
    Comma-separated list of standard HTML file input filters that restrict what
    files the user can pick from. Values can be:

    `<file extension>`:
        Specific file extension(s) (e.g: .gif, .jpg, .png, .doc) are pickable

    `audio/*`:
        all sound files are pickable

    `video/*`:
        all video files are pickable

    `image/*`:
        all image files are pickable

    `<media type>`:
        A valid `IANA Media Type`_, with no parameters.

    .. _IANA Media Type: https://www.iana.org/assignments/media-types/media-types.xhtml
    """)
        
    multiple = String(default="False", help="""
    set to "multiple" or "True" if selection of more than one file at a time
    should be supported.
    """)

some test code for bokeh-server:
(call bokeh serve test1.py )
test1.py:

from bokeh.io import curdoc
from bokeh.models.widgets import FileInput
from bokeh.layouts import column
import base64

def showfiles(attr,old, new):
    print("showfiles():")
    print(attr)
    if type(new) == list:
        for n in new:
            if attr == 'filename':
                print('Filename = %s ' % n)
            elif attr == 'mime_type':
                print('mime-type = %s ' % n)
            elif attr == 'value':
                print('contents= \n %s' % base64.decodebytes(n.encode()).decode('utf8'))
            else:
                print('unknown attribute %s' % attr)
    else:
        if attr == 'filename':
            print('Filename = %s ' % new)
        elif attr == 'mime_type':
            print('mime-type = %s ' % new)
        elif attr == 'value':
            print('contents= \n %s' % base64.decodebytes(new.encode()).decode('utf8'))
        else:
            print('unknown attribute %s' % attr)
    
# Original API   
file_input = FileInput() # This is for original bokeh 1.4

# New API
#file_input = FileInput(multiple='True') # this is for the modified FileInput widget

file_input.on_change('value', showfiles)
file_input.on_change('mime_type', showfiles)
file_input.on_change('filename', showfiles)

curdoc().add_root(column(file_input))

@ThomasB In general looks quite close. One change on the Python side: the help and defaults params should only go to the “top level” propert:

value = Either(String, List[String], default=..., help=...) 

Also I think the help strings could be improved by mentioning when the value is a single string and when it is a list.

I would suggest going ahead and making a PR at this point. Our other core dev @mateusz may have more suggestions on the JS side, but as long as you can do short round of iteration as needed, this should be an easy merge.

@Bryan Agreed, I was happy to have it running, though.
The PR will include improved help etc. .
It will be my very first PR, so be patient with me.

1 Like

Just opened

  • feature request Issue # 9727
  • associated PR #9728

(I have no clue about “automated testing” or CI etc., sorry! )

No worries @ThomasB we will help you get to the finish line :slight_smile: