allow current timestamp in save image prefix

[mcmonkey4eva]

Example:

Just set filename prefix to eg %year%-%month%-%day%/ComfyUI,
this will save as eg ComfyUI/output/2024-07-15/ComfyUI_00001_.png

Supercedes #3402 but with cleaner code and a more configurable usage

The benefit is, of course, more ways to organize files.

Arguably more vars would be useful, eg prompt and wotnot, but that would require text processing nodes linking together, which is possible with some custom node packs.
Date/time however is easily inserted directly.

I also added an if "%" in filename_prefix: guard to prevent redundant calculations when the user doesn't want them.

[shawnington]

This method is very prone to typos, and at the very least reducing it from %year% to %y% or something less prone to typos seems preferable, as this implementation requires manually typing out the path structure, it requires knowledge of the feature, which will not be readily apparent to new users that its a feature that even exists, without tool tips implemented.

I still am of the opinion that a default implementation that is not subject to typos is preferable, such as an implementation that has a list of format options that follow basic standard image management practices such as the nested format year/month/day/filename or year-month-day/filename

A single typo can potentially create a huge mess of a folder structure when done the way you suggest.

For example, if nested folders are desired and there is a typo in the form of %year%/%moth%/%day%/filename you just spawn a folder called moth that if not caught quickly can create an organizational mess, same applies to the format %year%-%moth%-%day%/filename

I don't see any benefit that is conferred by requiring manual entry over having a list that allows switching between standardized formats when the goal is to create a simple means of organizing images for users.

[shawnington]

A simple method like this is should be included in the implementation. It's simple, self documenting, user facing, and hardened against user error, but also allows the flexibility of your implementation to be used manually.

@classmethod
    def INPUT_TYPES(s):
        return {"required": 
                    {"images": ("IMAGE", ),
                     "filename_prefix": ("STRING", {"default": "ComfyUI"}),
                     "organize_by": (["none", "year/month/day", "year-month-day"],),
                     },
                "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, prompt=None, extra_pnginfo=None):
        if organize_by == "year-month-day":
            filename_prefix = "%year%-%month%-%day%" + "/" + filename_prefix
        if organize_by == "year/month/day":
            filename_prefix = "%year%/%month%/%day%" + "/" + filename_prefix

It can also easily be implemented without any changes to folder_paths.py like this, as this works with the current folder_paths.py implementation.

        if organize_by == "year-month-day":
            filename_prefix = time.strftime("%Y-%m-%d") + "/" + filename_prefix
        if organize_by == "year/month/day":
            filename_prefix = time.strftime("%Y/%m/%d") + "/" + filename_prefix

The structures that result from this implementation using the simple list selection look as follows:
year/month/day

year-month-day

[shawnington]

Another problem with the manual input method.

As currently implemented, available text space in the file name field is finite and including the organizational formatting in the file name causes clipping with quite short file names. This would also suggest that using %y%-%m%-%d%/filename is a better implementation, short of changing front end behavior of how the field space is calculated.

Resizing the node does not give more space.

[teward]

This method is very prone to typos, and at the very least reducing it from %year% to %y% or something less prone to typos seems preferable, as this implementation requires manually typing out the path structure, it requires knowledge of the feature, which will not be readily apparent to new users that its a feature that even exists, without tool tips implemented.

I would be very careful defining it as "y" because if we look at strftime type date identification codes (which follows the libc standard and the accepted strftime defaults according to that and Python's implementation) would get us 24 right now instead of 2024. It's also unclear what format someone wants to use if we are giving them such a short name.

I'd suggest that it'd be easier to allow people to define their own datetime formats, ala a %dt:FMTSTR or just %dt% that uses a default option based off of the user's locale/system (or uses the ISO standard for date definitions) and then apply it accordingly.

[shawnington]

This method is very prone to typos, and at the very least reducing it from %year% to %y% or something less prone to typos seems preferable, as this implementation requires manually typing out the path structure, it requires knowledge of the feature, which will not be readily apparent to new users that its a feature that even exists, without tool tips implemented.

I would be very careful defining it as "y" because if we look at strftime type date identification codes (which follows the libc standard and the accepted strftime defaults according to that and Python's implementation) would get us 24 right now instead of 2024. It's also unclear what format someone wants to use if we are giving them such a short name.

I'd suggest that it'd be easier to allow people to define their own datetime formats, ala a %dt:FMTSTR or just %dt% that uses a default option based off of the user's locale/system (or uses the ISO standard for date definitions) and then apply it accordingly.

Thats fair, it should follow a standard format, like strftime %Y, but a shortened standard that doesn't cause the filename to be truncated in the display would be ideal. Im assuming the reason for having the individual year-month-day is to allow flexibility in formatting so that you can do nested folders with year/month/day or even just do a dated file with year-month-day-filename or filename-year-month-day however you want to do it.

My preferred implementation is just a multiple format selection for user friendliness since this is useful enough that it should be a forward facing feature, not something that you need to dig through the documentation to find exists. However, having the ability to also do the custom formatting still included is also beneficial.

[teward]

My preferred implementation is just a multiple format selection for user friendliness since this is useful enough that it should be a forward facing feature, not something that you need to dig through the documentation to find exists. However, having the ability to also do the custom formatting still included is also beneficial.

I agree, having the ease of customization to extract the specific bits works great, but lets be careful how we name them. "%year%" is more understandable than "%y%" in my opinion because "plain text reading" is easier. Not opposed to a "common" set of options to use if you want to use %date% or such as a replacement. But, if all else fails, follow ISO standard (Year-Month-Day with zero-padding))

Note that I was inspired here by your PR to make #4037 which is not yet ready for review that lets people have much more control over the output format ala strftime formatting like I recommended. (Note that #4037 will stay in draft state until we determine how this one goes, so I can do a rebase and sync with master where needed.)

(Yes, I'm one of those people. Even if the wider user-base doesn't use it, it doesn't necessarily hurt to have the more complicated functionality available for those who want that fine-grained control).)

[shawnington]

My preferred implementation is just a multiple format selection for user friendliness since this is useful enough that it should be a forward facing feature, not something that you need to dig through the documentation to find exists. However, having the ability to also do the custom formatting still included is also beneficial.

I agree, having the ease of customization to extract the specific bits works great, but lets be careful how we name them. "%year%" is more understandable than "%y%" in my opinion because "plain text reading" is easier. Not opposed to a "common" set of options to use if you want to use %date% or such as a replacement. But, if all else fails, follow ISO standard (Year-Month-Day with zero-padding))

Note that I was inspired here by your PR to make #4037 which is not yet ready for review that lets people have much more control over the output format ala strftime formatting like I recommended. (Note that #4037 will stay in draft state until we determine how this one goes, so I can do a rebase and sync with master where needed.)

(Yes, I'm one of those people. Even if the wider user-base doesn't use it, it doesn't necessarily hurt to have the more complicated functionality available for those who want that fine-grained control).)

If we had to vote on a standard format, id go with the strftime way of %Y-%m-%d simply for the reason that most of the users who will be using the feature manually, will be familiar with python, and time is the most commonly used library for this in python. I also believe that format being shorter is beneficial owing to the finite field space.

[teward]

strftime way of %Y-%m-%d

AKA ISO 8601 format. strftime likes ISO 8601 too :) It is after all an International standard of time representation.

Agreed if we had a dropdown to select that'd be great, but we'd still need to define a 'standard' or use Python to query the current locale and use its date generation. I say this because we also see day-month-year formats and month-day-year formats depending on locales. ISO8601 exists for this reason.

Fully agree though it also helps having items like %year%, etc. to replace as well.

[shawnington]

strftime way of %Y-%m-%d

AKA ISO 8601 format. strftime likes ISO 8601 too :) It is after all an International standard of time representation.

Agreed if we had a dropdown to select that'd be great, but we'd still need to define a 'standard' or use Python to query the current locale and use its date generation. I say this because we also see day-month-year formats and month-day-year formats depending on locales. ISO8601 exists for this reason.

Fully agree though it also helps having items like %year%, etc. to replace as well.

I suggested localization in the original pr #3402 but it seems people want to adhere to unix standard 8601 format, and not localize. I like your PR idea of having a single short %dt to do date time for use in a way like filename-%dt in which I still think localization seems like a good idea for, as well as more granular control.

I agree 8601 for nested folders (although technically YYYY/MM/DD is not 8601, but would be processed by folder_paths.py as a nested file tree). Im agnostic about 8601 or localization for a folder that is just the current date, and I like localization as an option for inclusion in the file name like you suggest.

Definitely need a standard way of doing things going forward though so users know what to expect.

[teward]

I like your PR idea of having a single short %dt to do date time for use in a way like filename-%dt in which I still think localization seems like a good idea for, as well as more granular control.

Added to my PR in draft #4037 with %dt% being locale handled for 'automation' cases, with a caveat that if the 'locale' version has slashes it turns things into a folder structure as indicated, and %dt:iso% being a specific declarer of ISO 8601 format datetime. With %dt:FMT% (where FMT is a format string readable by strftime) to handle those specific cases, but I still agree with this PR that we should be interpreting things like %year%, etc. for more user-friendly controls.

Hope you don't mind me building up that with additional functionality as we discuss here :)

[shawnington]

I like your PR idea of having a single short %dt to do date time for use in a way like filename-%dt in which I still think localization seems like a good idea for, as well as more granular control.

Added to my PR in draft #4037 with %dt% being locale handled for 'automation' cases, with a caveat that if the 'locale' version has slashes it turns things into a folder structure as indicated, and %dt:iso% being a specific declarer of ISO 8601 format datetime. With %dt:FMT% (where FMT is a format string readable by strftime) to handle those specific cases, but I still agree with this PR that we should be interpreting things like %year%, etc. for more user-friendly controls.

Hope you don't mind me building up that with additional functionality as we discuss here :)

my only real issue with %year% is that the space for the filename is finite, and using it as indicated, truncates at 9 characters in the display, and navigating in the value modal when you change things, isn't much better. Using a standard format like %Y-%m-%d gives another 12 character of space for file naming. So this change should either include changes the expand the number of characters that can be displayed, or use a shorter format.

If the goal is user friendly, the selection list is the obvious solution. (see my updated output examples of what it actually does.)

[teward]

Using a standard format like %Y-%m-%d gives another 12 character of space for file naming

Unless we have a mechanism to detect what is or isn't a variable (current standard is %VAR% if you read the code) then %Y-%m-%d wouldn't properly parse if we adhere to the current 'standard' set in the code. We'd have to replace %Y which could be something we might not want to do without careful handling (because at what point do we determine %Y is just a variable or such? And do we really want to write a handler to parse all possible strftime variables in an input, which could cause other conflicts if something is added that has a conflict with %? (where ? here is any character)?)

A mini-middleground might be something like my PR which would do a format of %dt:%Y-%m-%d% which adheres to the current 'variable' standard (%VAR%) but adds customization capability within the variable string (painful but it works). Would that in your mind (user friendly dropdown select aside) be a middleground? (note that my dt implementation is extremely customizable by using strftime format strings/codes, because you could use %dt:%Y/%m/%d% and it'd be replaced with 2024/07/17 which would then parse as a folder structure for folder_paths and then behave that way.)

So this change should either include changes the expand the number of characters that can be displayed, or use a shorter format.

The question I have though is the "finite" limit. If we really want to save space for file naming, and allow customization to this extent, we need to make sure that whatever limit exists is actually sane, or whether we also need to expand that limit on the system. (Excluding the max file length that Windows sets, which is painfully annoying)

[shawnington]

Using a standard format like %Y-%m-%d gives another 12 character of space for file naming

Unless we have a mechanism to detect what is or isn't a variable (current standard is %VAR% if you read the code) then %Y-%m-%d wouldn't properly parse if we adhere to the current 'standard' set in the code. We'd have to replace %Y which could be something we might not want to do without careful handling (because at what point do we determine %Y is just a variable or such? And do we really want to write a handler to parse all possible strftime variables in an input, which could cause other conflicts if something is added that has a conflict with %? (where ? here is any character)?)

A mini-middleground might be something like my PR which would do a format of %dt:%Y-%m-%d% which adheres to the current 'variable' standard (%VAR%) but adds customization capability within the variable string (painful but it works). Would that in your mind (user friendly dropdown select aside) be a middleground? (note that my dt implementation is extremely customizable by using strftime format strings/codes, because you could use %dt:%Y/%m/%d% and it'd be replaced with 2024/07/17 which would then parse as a folder structure for folder_paths and then behave that way.)

So this change should either include changes the expand the number of characters that can be displayed, or use a shorter format.

The question I have though is the "finite" limit. If we really want to save space for file naming, and allow customization to this extent, we need to make sure that whatever limit exists is actually sane, or whether we also need to expand that limit on the system. (Excluding the max file length that Windows sets, which is painfully annoying)

Agreed on all points. The current issue isn't that it wont allow longer file lengths, it just truncates them in display, and resizing the node does not expand the number of characters that will be displayed. Combine that with the value editor also being incapable of showing a filename that includes those variables in full, and you have a really user unfriendly situation.

I haven't checked .js for the display, I am assuming it's just assigning it a width and overflow behavior on load and not refreshing on resize or something simple.