Set output default to subfolder of current date "%Y-%m-%d" to help better organize output.

[andrewtvuong]

The default behavior currently saves output to ./output as ComfyUI_XXXXX_.png, XXXXX being file counter. With prefix defaulted to ComfyUI. A thousand runs over a thousand days would all be sequential in one folder making it difficult to organize runs.

With discussion and contribution from @asagi4 to use ISO date format "%Y-%m-%d" and @shawnington way of toggling for default behavior.

Proposing a new default behavior where when toggled on for the SaveImage node, will use default subfolder with current date in "%Y-%m-%d" format, retaining behavior of saving ./output/"%Y-%m-%d"/prefix_XXXXX_.png. When toggled off will retain original default behavior of ./output/prefix_XXXXX_.png.

Looking like this: ➜ output git:(master) ✗ ls 2024-05-03 2024-04-18 output_images_will_be_put_here ➜ output git:(master) ✗ cd 2024-05-03 ➜ 2024-05-03 git:(master) ✗ ls ComfyUI_00001.png ComfyUI_00019_.png ComfyUI_00037_.png ComfyUI_00055_.png

Simple change but helps organization for default.

Thanks

[asagi4]

I think changing the node's default filename prefix to %date:yyyy-MM-dd%/ComfyUI would be a better way to do this (someone else needs to test if it works on Windows). I agree the current default isn't very good for organization.

If you do this, please don't put the month first. That's just a dumb way of formatting dates.

[shawnington]

Seems like localization should be used given differences in date-time formats globally.

[asagi4]

@shawnington I think localization would make things way too complex for the problem that it would solve. YYYY-MM-DD is the ISO standard and it also has the nice property of sorting correctly in alphabetical order, so you can easily sort your folders by time.

[andrewtvuong]

@shawnington I think localization would make things way too complex for the problem that it would solve. YYYY-MM-DD is the ISO standard and it also has the nice property of sorting correctly in alphabetical order, so you can easily sort your folders by time.

I agree with YYYY-MM-DD would work best for sort. I'll send a revision

[shawnington]

@shawnington I think localization would make things way too complex for the problem that it would solve. YYYY-MM-DD is the ISO standard and it also has the nice property of sorting correctly in alphabetical order, so you can easily sort your folders by time.

if os.normpath is called on on the date before os.join to create nested directories in the manner that something like Light Room organizes photos, ISO YYYY-MM-DD is definitely the preferred way to create a nested date structure for organizing.

However it appears to be creating individual folders for each date with the format, so it may still be preferable to create them in the format that is familiar to the user based on locality, as the intended purpose seems to be to aid quickly find images by date.

Fair point about sorting though.

[shawnington]

The more I think about it, this implementation needs quite a bit of work.

It discards subfolders specified in the SaveImage node in the format <sub_folder>/<file_prefix> by replacing this line

subfolder = os.path.dirname(os.path.normpath(filename_prefix))

with one that forces placement into a date folder while discarding the user specified subfolder.

subfolder = current_date

but leaving

filename = os.path.basename(os.path.normpath(filename_prefix))

because os.path.basename is called, this discards, any specified subfolder, and only leaves the filename which will be placed in the date folder even if a subfolder has been specified.

as overwriting the subfolder with the date, user specified folder are not returned to the SaveImage node in nodes.py as the date is joined into the path with out the user specified subfolder via:

full_output_folder = os.path.join(output_dir, subfolder)

and return to the SaveImage node where the image is saved via:

img.save(os.path.join(full_output_folder, file), pnginfo=metadata, compress_level=self.compress_level)

I do not believe discarding user specified subfolders is preferred behavior.

It seem there should be a check for user specified subfolders and perhaps default to place them there instead of in the date based folder if they are specified.

Perhaps modification of the SaveImage node to have an enable disable option to place the user specified subfolder inside the date folder, or in the base directory is a needed part of a full solution.

[asagi4]

Perhaps modification of the SaveImage node to have an enable disable option to place the user specified subfolder inside the date folder, or in the base directory is a needed part of a full solution.

I think just changing the default value of file_prefix in the nodes to include the directory would solve the most common usecase. Anyone who doesn't want date directories can just delete the directory part or otherwise modify it.

I think having it in as a default is better than not having it. It makes the format string discoverable and defaults to a bit more organized output folder.

[shawnington]

Perhaps modification of the SaveImage node to have an enable disable option to place the user specified subfolder inside the date folder, or in the base directory is a needed part of a full solution.

I think just changing the default value of file_prefix in the nodes to include the directory would solve the most common usecase. Anyone who doesn't want date directories can just delete the directory part or otherwise modify it.

I think having it in as a default is better than not having it. It makes the format string discoverable and defaults to a bit more organized output folder.

I think this is a palatable solution, but perhaps difficult to implement due to the way nodes are constructed.

the date would need obtained in the init of SaveImage in nodes.py, not in folder_paths.py

I am however not sure if format strings can be used in the INPUT_TYPES constructor to inject the date as part of the default value.

Perhaps something like this.

class SaveImage:
    def __init__(self):
        self.output_dir = folder_paths.get_output_directory()
        self.type = "output"
        self.prefix_append = ""
        self.compress_level = 4
        self.current_date = time.strftime("%Y-%m-%d")

    @classmethod
    def INPUT_TYPES(s):
        return {"required": 
                    {"images": ("IMAGE", ),
                     "filename_prefix": ("STRING", {"default": f"{s.current_date}"/ComfyUI"})},
                "hidden": {"prompt": "PROMPT", "extra_pnginfo": "EXTRA_PNGINFO"},
                }

Again, I am not sure if a variable can be injected into the INPUT_TYPES constructor, and I am not sure the current_date would be accessible via s, since s is an instance of cls in a class method, so this might not be working code which is why I suggest an enable/disable date option for placing subfolder inside or outside of the date folder.

[andrewtvuong]

Certainly don't intend on overriding user specified subfolders and that was my mistake, I was trying to set the default folder to be current date yyyy-mm-dd if none specified, so default, rather than defaulting to just output/.

Maybe we can check for filename_prefix if None, then replace with current_date, else no-op to preserve user specified. I imagine most new users would be better served with some organization than none. Then they can move onto non default, user specified dir.

    current_date = time.strftime("%Y-%m-%d")
    filename_prefix = compute_vars(filename_prefix, image_width, image_height)

    if filename_prefix:
        subfolder = os.path.dirname(os.path.normpath(filename_prefix))
    else:
        subfolder = current_date
    filename = os.path.basename(os.path.normpath(filename_prefix))

    full_output_folder = os.path.join(output_dir, subfolder)
    

How's that sound @shawnington @asagi4

[shawnington]

Certainly don't intend on overriding user specified subfolders and that was my mistake, I was trying to set the default folder to be current date yyyy-mm-dd if none specified, so default, rather than defaulting to just output/.

Maybe we can check for filename_prefix if None, then replace with current_date, else no-op to preserve user specified. I imagine most new users would be better served with some organization than none. Then they can move onto non default, user specified dir.

How's that sound @shawnington @asagi4

That seems okay to me.

Id personally also want an enable disable toggle added to the SaveImage node by a simple name such as "organize by date" which can be enable by default, to allow users that wish to specify a custom folder, to be able to have it placed inside the date folder.

It just seems if this functionality is added, it should not exclude users who wish to specify a subfolder for the image to go into.

That would seem to require changing the folder_paths.py to either return the constituent path elements or state of the organize by date option, so that joining can be done properly.

My preferred default behavior for your suggested change:

• organize by date option: default = enabled
• no user path specified -> into date folder
• user path specified but date folder enabled -> place user specified folder in date folder
• user path specified but place in date folder disabled -> place in default directory in specified folder

[andrewtvuong]

That seems okay to me.

Id personally also want an enable disable toggle added to the SaveImage node by a simple name such as "organize by date" which can be enable by default, to allow users that wish to specify a custom folder, to be able to have it placed inside the date folder.

I added the minimal change for defaulting to current date instead of none.

I do like the toggle idea, brings more visibility, but I think that can built upon minimal change. We can follow up with it.

Thanks for yalls feedback.

[shawnington]

Certainly don't intend on overriding user specified subfolders and that was my mistake, I was trying to set the default folder to be current date yyyy-mm-dd if none specified, so default, rather than defaulting to just output/.

Maybe we can check for filename_prefix if None, then replace with current_date, else no-op to preserve user specified. I imagine most new users would be better served with some organization than none. Then they can move onto non default, user specified dir.

    current_date = time.strftime("%Y-%m-%d")
    filename_prefix = compute_vars(filename_prefix, image_width, image_height)

    if filename_prefix:
        subfolder = os.path.dirname(os.path.normpath(filename_prefix))
    else:
        subfolder = current_date
    filename = os.path.basename(os.path.normpath(filename_prefix))

    full_output_folder = os.path.join(output_dir, subfolder)
    

How's that sound @shawnington @asagi4

This if statement would always return true, as filename_prefix has a default value, you would need to do the os.path.dirname first as the splits out the user defined subfolder, and check based on that

[asagi4]

The filename prefix will never be None because the nodes all have defaults for it. What I'm talking about it just doing this:

index 50deb29..bf7f44b 100644
--- a/nodes.py
+++ b/nodes.py
@@ -1387,7 +1387,7 @@ class SaveImage:
     def INPUT_TYPES(s):
         return {"required": 
                     {"images": ("IMAGE", ),
-                     "filename_prefix": ("STRING", {"default": "ComfyUI"})},
+                     "filename_prefix": ("STRING", {"default": "%date:yyyy-MM-dd%/ComfyUI"})},
                 "hidden": {"prompt": "PROMPT", "extra_pnginfo": "EXTRA_PNGINFO"},
                 }

and the same for other nodes that save files.

[shawnington]

The filename prefix will never be None because the nodes all have defaults for it. What I'm talking about it just doing this:

index 50deb29..bf7f44b 100644
--- a/nodes.py
+++ b/nodes.py
@@ -1387,7 +1387,7 @@ class SaveImage:
     def INPUT_TYPES(s):
         return {"required": 
                     {"images": ("IMAGE", ),
-                     "filename_prefix": ("STRING", {"default": "ComfyUI"})},
+                     "filename_prefix": ("STRING", {"default": "%date:yyyy-MM-dd%/ComfyUI"})},
                 "hidden": {"prompt": "PROMPT", "extra_pnginfo": "EXTRA_PNGINFO"},
                 }

and the same for other nodes that save files.

That is a simple way to do it if it works, Im still not sure on if injecting the date into the INPUT_TYPES breaks anything, did you test this to see if it works?

It can also be simply done with an intuitive toggle like this.

@classmethod
    def INPUT_TYPES(s):
        # format the string to include variable self.type
        return {"required": 
                    {"images": ("IMAGE", ),
                     "filename_prefix": ("STRING", {"default": "ComfyUI"}),
                     "organize_by_date": ("BOOLEAN", {"default": True, "label_on": "enable", "label_off": "disable"}),},
                "hidden": {"prompt": "PROMPT", "extra_pnginfo": "EXTRA_PNGINFO"},
                }

    RETURN_TYPES = ()
    FUNCTION = "save_images"

    OUTPUT_NODE = True

    CATEGORY = "image"

    def save_images(self, images, filename_prefix="ComfyUI", organize_by_date=True, prompt=None, extra_pnginfo=None):
        if organize_by_date:
            filname_prefix = time.strftime("%Y-%m-%d") + "/" + filename_prefix

I find this method preferable because it doesn't crowd the text box, or leave an opportunity to make errors when deleting the date or inserting a subfolder.

Just tested locally, no changes are required to folder_paths.py

It just simply prepend the date folder to the front of the file_prefix if the option is enabled.

I think all 3 solutions we have come up with are reasonable ways to tackle the problem.

[andrewtvuong]

Pesky Windows '\' :( does this impact anything if the change is done through nodes.py?

Feel free to push your updates yall. I'm good either way, much better my initial commit lol.

        if organize_by_date:
            filname_prefix = time.strftime("%Y-%m-%d") + "/" + filename_prefix

Typo in var name

[shawnington]

Pesky Windows \ :( does this impact anything if the change is done through nodes.py?

The slash direction shouldn't matter, as the os.path.normpath(filename_prefix) in the folder_path.py should change the slashes to be compatible with what ever OS is it is running on.

Someone on a different platform would need to cross check and verify that this is indeed the case.

[shawnington]

Pesky Windows '' :( does this impact anything if the change is done through nodes.py?

Feel free to push your updates yall. I'm good either way, much better my initial commit lol.

        if organize_by_date:
            filname_prefix = time.strftime("%Y-%m-%d") + "/" + filename_prefix

Typo in var name

Yep, I typed the code into the comment before I retyped it into the local version, but feel free to add it to your pull request, no need to have multiple requests open for the same thing.

[andrewtvuong]

@shawnington Latest commit used your changes to nodes.py https://github.com/comfyanonymous/ComfyUI/pull/3402#issuecomment-2094914458 works great!

Just tested. I did not know how to do the toggling before, but it was a clean and good change. Thanks!

[shawnington]

@shawnington Latest commit used your changes to nodes.py #3402 (comment) works great!

Just tested. I did not know how to do the toggling before, but it was a clean and good change. Thanks!

If it's not merged into the main branch, you should offer it as a custom node, seems like a worthwhile effort. It a good idea, and Id use it.

You can also expand the options to separate by year month or day, and changing the format from YYYY-MM-DD to YYYY/MM/DD etc, for the different options. The folder_path.py handles the separations as long as a slash is included in the file_prefix path, it will be a new level of nesting.

Code looks good to me.

[andrewtvuong]

Hi @asagi4 @shawnington @comfyanonymous just updated the original body with the final changes proposed. Thanks for the discussions, contributions, and review.

[mcmonkey4eva]

Adding a new organize_by_date option to a core node's Required inputs list could break external usages - it should ideally be moved to optional, or perhaps change the implementation to instead just be an edit to the default prefix into ComfyUI/%year%-%month%-%day%, and then edit folder_paths.py get_save_image_path's compute_vars to have time vars - this would then also as discussed above allow users to change the format to a different one they prefer

[andrewtvuong]

Adding a new organize_by_date option to a core node's Required inputs list could break external usages - it should ideally be moved to optional, or perhaps change the implementation to instead just be an edit to the default prefix into ComfyUI/%year%-%month%-%day%, and then edit folder_paths.py get_save_image_path's compute_vars to have time vars - this would then also as discussed above allow users to change the format to a different one they prefer

Hi @mcmonkey4eva thanks for the callout, if we had to make this change, I would prefer the latter to keep things simpler by changing the default to %year%-%month%-%day%/ComfyUI, this would segregate the files by date sessions.

After 2 months, I've really enjoyed these changes, has vastly improved my workflow.

[andrewtvuong]

Hi @mcmonkey4eva @shawnington please take a look at what you think of the new changes that removes organize_by_date and defaults to "%year%-%month%-%day%/" + filename_prefix

https://github.com/comfyanonymous/ComfyUI/pull/3402/commits/0f3592dbd581ea11b1345dd10d384a55d57122bf

[andrewtvuong]

My bad yall for overcomplicating this.

I have moved the organize_by_date to optional from required like our original implementation @shawnington but with @mcmonkey4eva suggestion to remove from required and put in optional instead.

I think this is the best approach. I also realized, there was no way to disable the organize by date when I sent the previous commit.

https://github.com/comfyanonymous/ComfyUI/pull/3402/commits/db79828c960bea9f2b16f1a9be8df5e403d50896

Sleep coding bad... zzzz

[mcmonkey4eva]

I strongly encourage you to put it back to the compute_vars setup

[andrewtvuong]

I strongly encourage you to put it back to the compute_vars setup

Made the change! Left a comment on the guard suggestion, wasn't sure if this is what you meant.

[mcmonkey4eva]

organize_by_date should not be a var here at all

[mcmonkey4eva]

yknow what i'll just https://github.com/comfyanonymous/ComfyUI/pull/4030 to save the trouble