lucafoscili / comfyui-lf

Custom nodes with a touch of extra UX ✨ History for primitives, JSON manipulation, logic switches with visual feedback, LLM chat, analytics nodes, CivitAI metadata fetching... and more!
MIT License
31 stars 3 forks source link

'Convert string to JSON' node takes input as a list? #87

Closed Jonseed closed 1 week ago

Jonseed commented 1 week ago

I've been struggling to get the convert string to JSON node to work. It was outputting a list with a JSON dict inside it, but this was not being handled correctly by other nodes that expect a JSON object input such as get value from JSON. When I put the class into ChatGPT and asked about it, it said that the problem was that the class was treating the input as a list with the variable INPUT_IS_LIST = True so it was outputting a list with a dict inside. I changed this variable to False, and now it seems to be working correctly, inputting a string and outputting a JSON object. Do you know why INPUT_IS_LIST in this class is set to True?

lucafoscili commented 1 week ago

Hi, the management of lists in that node is relatively new, it's likely there's something to fix.

I needed to handle lists for this workflow:

ConvertImagesForWeb

When the input is a list of strings, the output should be an array of objects. Can you show me the workflow you are using the node into to better understand the use-case? :)

Jonseed commented 1 week ago

Here is an example of the workflow I was doing. The stringified JSON should just be converted to JSON, but fails on the node of get value from JSON because that node receives a list object (JSON array), not a JSON object (dictionary), as you can see in the display JSON. The json.get(key, None) method is being called on a list object but it expects a dictionary object.

It seems convert string to JSON should just convert a simple string to a JSON object. Maybe there could be a separate node for converting a list of strings to a JSON array? And maybe the get value from JSON node needs to be able to handle not only JSON objects but JSON arrays?

ComfyUI_00001_

Here is the error:

!!! Exception during processing !!! 'list' object has no attribute 'get'
Traceback (most recent call last):
  File "D:\repos\ComfyUI\execution.py", line 323, in execute
    output_data, output_ui, has_subgraph = get_output_data(obj, input_data_all, execution_block_cb=execution_block_cb, pre_execute_cb=pre_execute_cb)
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\repos\ComfyUI\execution.py", line 198, in get_output_data
    return_values = _map_node_over_list(obj, input_data_all, obj.FUNCTION, allow_interrupt=True, execution_block_cb=execution_block_cb, pre_execute_cb=pre_execute_cb)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\repos\ComfyUI\execution.py", line 169, in _map_node_over_list
    process_inputs(input_dict, i)
  File "D:\repos\ComfyUI\execution.py", line 158, in process_inputs
    results.append(getattr(obj, func)(**inputs))
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\repos\ComfyUI\custom_nodes\comfyui-lf\modules\nodes\json.py", line 74, in on_exec
    value = json.get(key, None)
            ^^^^^^^^
AttributeError: 'list' object has no attribute 'get'
lucafoscili commented 1 week ago

Oh I see, this evening I'll push a fix :)

I'll edit the output to be a [] only when:

It should work like this!

Also yes, it would be good to safeguard the getvalue node to handle lists, maybe by returning the value of the first element (?). It could also return the value of the first object that actually has that property, but I think it's a far-fetched scenario.

EDIT: Btw, I think that for an use-case like the one you sent, using the WriteJSON node would be more handy: workflow (1)

Jonseed commented 1 week ago

Thanks. My workflow is actually much more complex, and I'm taking a string from another node that I need to convert to JSON, and not writing it directly in JSON. That's why I needed your convert node.

Maybe the get value from JSON node could have the option of selecting which element of a JSON array to use to get the key, and default to the first element (or none if it is a simple JSON object input).

lucafoscili commented 1 week ago

All done, let me know if it's okay now!

Jonseed commented 1 week ago

Looks like it is working great! Thanks!