Some macros improvement help

[Andre601]

I've made a macros that automatically generates the necessary HTML in a page to depict crafting recipes based on data retrieved via a JSON file.

While the stuff works as intended do I know for a fact that my macros isn't the most efficient or easy to follow thing (primarely due to my lack of Python coding knowledge), so I now ask for some help in improving it wherever possible.

I first want to give context of what the macros generates.
Here is an example HTML that it generates:

<div class="md-typeset__table">
  <table>
    <thead>
      <tr>
        <th>Ingredients</th>
        <th>Crafting recipe</th>
      </tr>
    </thead>
    <tbody>
      <tr>
        <td>Block of Aluminum</th>
        <td>
          <div class="crafting-table tooltips">
            <span class="invslot-item slot0" data-minetip-title="Aluminum Ingot" data-minetip-text="<gray>Can be used to craft items">
              <img src="/VanillaPlus/assets/img/items/aluminum_ingot.png" alt="">
              <div class="quantity">9</div>
            </span>
            <span class="invslot-item slot1" data-minetip-title="Block of Aluminum">
              <img src="/VanillaPlus/assets/img/items/aluminum_ingot.png" alt="">
            </span>
            <span class="invslot-item slot2"></span>
            <span class="invslot-item slot3"></span>
            <span class="invslot-item slot4"></span>
            <span class="invslot-item slot5"></span>
            <span class="invslot-item slot6"></span>
            <span class="invslot-item slot7"></span>
            <span class="invslot-item slot8"></span>
            <span class="invslot-item slot9"></span>
            <img src="/VanillaPlus/assets/img/recipes/arrow.png" alt="" class="arrow" draggable="false">
            <span class="shapeless" data-minetip-title="This recipe is shapeless">
              <img src="/VanillaPlus/assets/img/recipes/shapeless.png" alt="" draggable="false">
            </span>
          </div>
        </td>
      </tr>
    </tbody>
  </table>
</div>

And this would be the resulting content:

Finally is here the full macros.py file I currently have:

import posixpath
import json

def define_env(env):
    
    @env.macro
    def recipe(ingredients, item: str):
        if not item:
            return "ERROR: No Item Path specified"
        
        json_data = read_json("docs/assets/data.json")
        
        if not "recipes" in json_data:
            return "ERROR: No recipe section found in data.json!"
        
        if not "items" in json_data:
            return "ERROR: No 'items' section found in data.json!"
        
        recipes = json_data["recipes"]
        if not item in recipes:
            return f"ERROR: No Recipe for item '{item}' found!"
        
        recipe = recipes[item]
        if not "result" in recipe:
            return f"ERROR: No result item defined for '{item}' recipe."
        
        items = json_data["items"]
        
        result = "<div class=\"md-typeset__table\"><table><thead><tr><th>Ingredients</th><th>Crafting recipe</th></tr></thead><tbody><tr><td>"
        ingredients_str = ""
        for ingredient in ingredients:
            if ingredients_str:
                ingredients_str += " + "
            
            ingredients_str += ingredient
        
        result += ingredients_str + "</td><td>"
        result += f"""
<div class="crafting-table tooltips">
{get_slot_html(recipe["slot1"], items, 1)}
{get_slot_html(recipe["slot2"], items, 2)}
{get_slot_html(recipe["slot3"], items, 3)}
{get_slot_html(recipe["slot4"], items, 4)}
{get_slot_html(recipe["slot5"], items, 5)}
{get_slot_html(recipe["slot6"], items, 6)}
{get_slot_html(recipe["slot7"], items, 7)}
{get_slot_html(recipe["slot8"], items, 8)}
{get_slot_html(recipe["slot9"], items, 9)}
<img src="/VanillaPlus/assets/img/recipes/arrow.png" class="arrow" alt="" draggable="false">
<span class="shapeless" {'data-minetip-title="This recipe is shapeless"' if "shapeless" in recipe and recipe['shapeless'] else ""}>{'<img src="/VanillaPlus/assets/img/recipes/shapeless.png" alt="" draggable="false">' if "shapeless" in recipe and recipe['shapeless'] else ""}</span>
{get_slot_html(recipe["result"], items, 0, recipe["quantity"] if "quantity" in recipe else 1)}
</div>
</td></tr></tbody></table></div>
"""
        return result
    
    def get_slot_html(item: str, json_data, slot: int, quantity: int = 1):
        item_data = json_data[item] if item else None
        if not item_data:
            return f"<span class=\"invslot-item slot{slot}\"></span>"
        
        result = f"<span class=\"invslot-item slot{slot}\" data-minetip-title=\""
        result += item_data["title"] if "title" in item_data else "Unknown Item"
        result += "\""
        if "lore" in item_data:
            result += f" data-minetip-text=\"{item_data['lore']}\""
        result += "><img src=\"/VanillaPlus/assets/img/"
        result += item_data["img"] if "img" in item_data else "blocks/unknown"
        result += ".png\" alt=\"\">"
        if quantity > 1:
            result += f'<div class="quantity">{quantity}</div>'
        result += "</span>"
        
        return result
    
    def read_json(file_path: str):
        """
        Reads a provided JSON path and converts it to a usable Jinja2 object structure.
        """
        path = posixpath.sep.join([env.project_dir, file_path])
        with open(path, 'r', encoding='utf-8') as file:
            return json.load(file)

Apreciate any help to improve this mess.
I'm also open for help with the JS and CSS used (This stuff isn't from me, so I have even less ideas what can/should be improved). Let me know if you want to see that stuff too.

Edit

Adding the JSON used as well, so that you can see the structure it has. Preferrably would I like to be able to remove as much as possible (i.e. be able to ommit empty values altogether).

{
    "items": {
        "aluminum_block": {
            "title": "Block of Aluminum",
            "img": "blocks/aluminum_block"
        },
        "aluminum_ingot": {
            "title": "Aluminum Ingot",
            "img": "items/aluminum_ingot"
        },
        "aluminum_hammer": {
            "title": "Aluminum Hammer",
            "img": "items/aluminum_hammer"
        },
        "amethyst_shard": {
            "title": "Amethyst Shard",
            "img": "items/amethyst_shard"
        },
        "copper_boots": {
            "title": "Copper Boots",
            "img": "items/copper_boots"
        },
        "copper_chestplate": {
            "title": "Copper Chestplate",
            "img": "items/copper_chestplate"
        },
        "copper_helmet": {
            "title": "Copper Helmet",
            "img": "items/copper_helmet"
        },
        "copper_ingot": {
            "title": "Copper Ingot",
            "img": "items/copper_ingot"
        },
        "copper_leggings": {
            "title": "Copper Leggings",
            "img": "items/copper_leggings"
        },
        "crushed_ender_pearl": {
            "title": "Crushed Ender Pearl",
            "img": "items/crushed_ender_pearl"
        },
        "ender_pearl": {
            "title": "Ender Pearl",
            "img": "items/ender_pearl"
        },
        "raw_bauxite": {
            "title": "Raw Bauxite",
            "img": "items/raw_bauxite"
        },
        "raw_bauxite_block": {
            "title": "Block of raw Bauxite",
            "img": "blocks/raw_bauxite_block"
        },
        "stick": {
            "title": "Stick",
            "img": "items/stick"
        },
        "teleport_crystal": {
            "title": "Teleport Crystal",
            "lore": "<gray>Teleports you to a\n<gray>previously saved location.\n\n<gray>Requires <aqua>1 XP Level</aqua> to\n<gray>save your location.\n\n<aqua>1st Right Click: <gray>Save Location.\n<aqua>2nd Right Click: <gray>Teleport.",
            "img": "items/teleport_crystal"
        }
    },
    "recipes": {
        "aluminum_block": {
            "result": "aluminum_block",
            "slot1": "aluminum_ingot",
            "slot2": "aluminum_ingot",
            "slot3": "aluminum_ingot",
            "slot4": "aluminum_ingot",
            "slot5": "aluminum_ingot",
            "slot6": "aluminum_ingot",
            "slot7": "aluminum_ingot",
            "slot8": "aluminum_ingot",
            "slot9": "aluminum_ingot"
        },
        "aluminum_hammer": {
            "result": "aluminum_hammer",
            "slot1": "",
            "slot2": "aluminum_ingot",
            "slot3": "",
            "slot4": "",
            "slot5": "stick",
            "slot6": "aluminum_ingot",
            "slot7": "stick",
            "slot8": "",
            "slot9": ""
        },
        "aluminum_ingot": {
            "result": "aluminum_ingot",
            "slot1": "aluminum_block",
            "slot2": "",
            "slot3": "",
            "slot4": "",
            "slot5": "",
            "slot6": "",
            "slot7": "",
            "slot8": "",
            "slot9": "",
            "shapeless": true,
            "quantity": 9
        },
        "copper_boots": {
            "result": "copper_boots",
            "slot1": "copper_ingot",
            "slot2": "",
            "slot3": "copper_ingot",
            "slot4": "copper_ingot",
            "slot5": "",
            "slot6": "copper_ingot",
            "slot7": "",
            "slot8": "",
            "slot9": ""
        },
        "copper_chestplate": {
            "result": "copper_chestplate",
            "slot1": "copper_ingot",
            "slot2": "",
            "slot3": "copper_ingot",
            "slot4": "copper_ingot",
            "slot5": "copper_ingot",
            "slot6": "copper_ingot",
            "slot7": "copper_ingot",
            "slot8": "copper_ingot",
            "slot9": "copper_ingot"
        },
        "copper_helmet": {
            "result": "copper_helmet",
            "slot1": "copper_ingot",
            "slot2": "copper_ingot",
            "slot3": "copper_ingot",
            "slot4": "copper_ingot",
            "slot5": "",
            "slot6": "copper_ingot",
            "slot7": "",
            "slot8": "",
            "slot9": ""
        },
        "copper_leggings": {
            "result": "copper_leggings",
            "slot1": "copper_ingot",
            "slot2": "copper_ingot",
            "slot3": "copper_ingot",
            "slot4": "copper_ingot",
            "slot5": "",
            "slot6": "copper_ingot",
            "slot7": "copper_ingot",
            "slot8": "",
            "slot9": "copper_ingot"
        },
        "raw_bauxite_block": {
            "result": "raw_bauxite_block",
            "slot1": "raw_bauxite",
            "slot2": "raw_bauxite",
            "slot3": "raw_bauxite",
            "slot4": "raw_bauxite",
            "slot5": "raw_bauxite",
            "slot6": "raw_bauxite",
            "slot7": "raw_bauxite",
            "slot8": "raw_bauxite",
            "slot9": "raw_bauxite"
        },
        "teleport_crystal": {
            "result": "teleport_crystal",
            "slot1": "crushed_ender_pearl",
            "slot2": "amethyst_shard",
            "slot3": "",
            "slot4": "",
            "slot5": "",
            "slot6": "",
            "slot7": "",
            "slot8": "",
            "slot9": "",
            "shapeless": true
        }
    }
}

[fralau]

Hi @Andre601,

Your code is quite understandable. There are not a lot of lines to generate in your output string so I don't think that "optimizing" code execution is really an issue.

Perhaps, if you want to make your code more "pythonic", you can make incremental improvements:

• Use simple quotes instead of double quotes (') or three quotes around strings (''' or """), to avoid escaping double quotes (which happen a lot in css).
• Since strings are immutable, adding to strings is not ideal. What you could do, is to create a list of strings, and finally join it. That would give more structure to your code.

strings =  [f"<span class=\"invslot-item slot{slot}\" data-minetip-title=\"", 
                 item_data["title"] if "title" in item_data else "Unknown Item",
                '\"']
result = "\n".join(strings)

• While you create your list (before joining it), use a for loop when needed:

for num in range(1, 10):
    strings.append("""{get_slot_html(recipe[f"slot{num}"], items, num)}""")

I hope this helps.

[Andre601]

Regarding the for loop. Slots are slot0 up to slot9, so I assume I would change the values of range to 0, 9?
Also, Slot0 in my case is also using a different key in the JSON file (result), but I assume I could just do a check for if num is 0 to handle it differently.

Finally, is there a way to make the JSON keys optional? Right now am I required to include all slot{n} keys, even if their values would be empty and I would like to change that to reduce the overall length of the file.

Also, not sure if that's a typo on your end, but shouldn't the """ string in your for-loop example have a f prefixed for variables to be used? Or is that not necessary?

[Andre601]

Actually, range would be 0, 10 since it seems to be i, i+1, i+2, ... j-1, aka start is inclusive and end is exclusive.

[fralau]

@Andre601 Yes, if your loop starts from zero, you could write range(10), from 0 to 9.

In general, I suggested code without running it, so there might be some formal errors (you should be able to fix them easily).

JSON is just data, so of course your code could omit empty slots (and assign default values). I guess the idiom you are looking for is this one. For example:

value = my_dict.get(key, default_value)

Contrary to my_dict[key], the get method doesn't fail, but returns a default value (None if you don't specify anything).

See more information here. This website is a good resource if you want to gradually improve your Python coding skills.

[fralau]

💡 As a general note, it is good news that writing macros for Mkdocs can be a motivation for learning Python.