unformatted {pic} string in node_details.json

[hyanwong]

I'm seeing json return values like:

{"colnames_nodes":{"id":0,"ott":1,"popularity":2,"age":3,"name":4,"iucnNE":5,"iucnDD":6,"iucnLC":7,"iucnNT":8,
"iucnVU":9,"iucnEN":10,"iucnCR":11,"iucnEW":12,"iucnEX":13,"{pic}1":14,"{pic}2":15,"{pic}3":16,"{pic}4":17,"{pic}5":18,"{pic}6":19,"{pic}7":20,"{pic}8":21}, "colnames_leaves":{"id":0,"ott":1,"popularity":2,"name":3,"extinction_date":4,"price":5},
"colnames_images":{"ott":0,"src_id":1,"src":2,"rating":3},"colnames_reservations":{"OTT_ID":0,"verified_kind":1,"verified_name":2,"verified_more_info":3,"verified_url":4},"nodes":[],"leaves":[],"lang":"en-GB,en;q=0.9","vernacular_by_ott":[],"vernacular_by_name":[],"leafIucn":[],"leafPic":[],"reservations":[],"tours_by_ott":[]}

I'm pretty sure that the "{pic}1" type keys are errors in string substitution, where we have forgotten the f in front of the string.

[davidebbo]

Seems related to this comment in OZfunc.py:

    # TODO - there is a bug here where we don't actually substitute {pic} into the name
    pic_ncols = ["{pic}1","{pic}2","{pic}3","{pic}4","{pic}5","{pic}6","{pic}7","{pic}8"]

I see the same on https://www.onezoom.org/. So maybe not a new issue? Does it result in something breaking?

[hyanwong]

Yep, I think that's the one. I forgot I had commented about that!

It doesn't break anything, no. But I guess this is a good time to fix it. This call happens a lot, so extra bytes will add up.

[lentinj]

If you're fixing it, don't forget the other end:

https://github.com/OneZoom/OZtree/blob/fb96f14ff9802aaa25827f0aeb7e0180a450bb38/OZprivate/rawJS/OZTreeModule/src/factory/data_repo.js#L291-L298

[hyanwong]

Perfect, thanks for the hint @lentinj

[hyanwong]

I'm inclined to make this "p1", "p2", etc, to cut down on bytes transferred, or "im1", "im2", etc which is maybe more obvious from the outside?

[davidebbo]

So you mean regardless of whether we're using rep, rtr or rpd, the column names returned to the client would be the same? i.e. not match the underlying DB cols that we're using?

[lentinj]

I'm inclined to make this "p1", "p2", etc,

I'd prefer "pic1", for naming consistencies' sake. Lots of internal functions / variables in JS-land use "pic" as a prefix.

[hyanwong]

So you mean regardless of whether we're using rep, rtr or rpd, the column names returned to the client would be the same? i.e. not match the underlying DB cols that we're using?

Good point. The JS doesn't seem to know at the moment about the rep, rtr, rpd values, but I could see the value of switching so that we match the DB (and can e.g. cache the different images). What do you think @davidebbo ?

[hyanwong]

I'm inclined to do as we obviously intended and change these to rpd1, rpd2 if they are pd images, rtr1 etc if they are trusted, etc, to reflect the DB. But I guess that would require some JS hacking.

[davidebbo]

Good point. The JS doesn't seem to know at the moment about the rep, rtr, rpd values, but I could see the value of switching so that we match the DB (and can e.g. cache the different images). What do you think @davidebbo ?

The risk is that doing this will turn this into a more complex change. e.g. in the data_repo.js code that @lentinj points to, it hard codes the column name, and would need to know what name to use based on context? I don't have enough understanding on how we make use of the picture column names on the client side.

[hyanwong]

Yep, it's definitely more complicated, but I think worth looking at. It seems wrong that the JS has no idea of the type of image, and I think if could be the source of some subtle bugs to do with which is displayed.

This is almost certainly something to discuss with @jrosindell. It doesn't need to be fixed for v4.0

[davidebbo]

in addition to data_repo.js above, we'd likely need to change:

API.py: node_image_cols = [node_cols["{pic}"+str(x+1)] for x in range(8)]

col_headers.js: let col_headers = [etc...] {pic}1":14,"{pic}2":15,"{pic}3":16,"{pic}4":17,"{pic}5":18,"{pic}6":19,"{pic}7":20, [etc...]

Anyway, since this is not new, and not blatantly broken, it can probably wait as you suggest.