Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Directory helper nodes #606

Closed
joeyballentine opened this issue Jul 21, 2022 · 14 comments · Fixed by #2702
Closed

Directory helper nodes #606

joeyballentine opened this issue Jul 21, 2022 · 14 comments · Fixed by #2702
Labels
enhancement New feature or request

Comments

@joeyballentine
Copy link
Member

It would be nice if we had a good way of splitting and combining directories. Like being able to do os.path.join essentially, or splitting a directory into its different parts.

Maybe this just means we need a way to convert a string to a directory, so that Text Append or Text Pattern would be able to be used for a directory

@RunDevelopment
Copy link
Member

This could become difficult.

What a valid directory is depends on the OS. Even simply joining two paths is non-trivial, since different operating systems have different separators. So it would be nice to have a simple cross-platform API (nodes).

@joeyballentine
Copy link
Member Author

Yeah that's why I was thinking of having nodes for it. And if we did allow text to path stuff it would have to go through a node first to transform it to a proper path

@RunDevelopment
Copy link
Member

I'm somewhat against going the paths-are-strings route so many programming languages go. While all paths are string, not all strings are paths. However, I also see that using strings is significantly simpler, from both a user perspective and our perspective.

I just worry about the potential for accidental bugs.

@joeyballentine
Copy link
Member Author

If we can come up with a good way to not use strings, I'm fine with that. But we do need to have some way to append strings onto paths at least

@joeyballentine joeyballentine added the enhancement New feature or request label Jul 31, 2022
@Athari
Copy link

Athari commented Oct 6, 2022

What a valid directory is depends on the OS. Even simply joining two paths is non-trivial, since different operating systems have different separators. So it would be nice to have a simple cross-platform API (nodes).

All operating systems support "/" separator. Yes, that includes Windows, with Batch scripts, PowerShell and everything else. Some Windows applications may have issues with forward slashes, but on the Windows API level, slashes are treated equally.

Path validation is largely irrelevant because input values would be existing file and directory names (input directory, image file name, model file name etc.) and user-provided strings within the node graph. There're no metadata extraction nodes (which may be the source of problematic paths) and I doubt they'll be added soon.

What doesn't exist is a simple cross-platform API. Trying to account for all cases is guaranteed to result in a total mess: file extensions, permission bits, drives etc.

I'd say having something, possibly hacky, to manipulate paths, is better than having nothing. So treating paths as strings with forward slahes and adding several text/path transform nodes would cover 90% of cases (frankly, just regex replace would cover 90% of my cases). And when the time comes for proper API, paths coould be treated as arrays of path elements or whatever, with an option of treating them as strings for backward compatibility.

@RunDevelopment
Copy link
Member

RunDevelopment commented Oct 7, 2022

In the future (and even in the now), we want chains to be easy to share. We already plan on adding a feature that allows you to create custom nodes by combining existing nodes in chainner (basically like a library from other programming languages). So getting cross-platform compatibility right is quite important.

I agree that using / as a path separator everywhere would probably be the best solution, however that also comes with the twist. Firstly, we should ensure that chainner users can believe the comfortable lie that all paths use /. So all paths that the user can observe must have their path separator replaced with /. However, do we allow users the input (relative) paths containing \ then? I believe we should. When parsing input paths (or path segments), we should treat both / and \ as the path separator and replace both with /.


Alright, path separator solved. Which format/protocol do we use?

This might be a bit of a weird question, but I believe that it would be useful to us to use URLs instead of file paths. URLs are a super set of file paths (file:///...) and it's pretty easy to convert between file URLs and file paths. However, URLs also allow us to make chains more powerful. User could easily embed remote files, and we could even define chainner-specific schema.
E.g. we talked about a download page for the models here. Instead of having a path to the downloaded model, we could use a special URL, e.g. chainner://model.game-upscale/<model-id>, that describes which model to use instead of which model file. This would allow us to then automatically download models as needed. Most of our users use the game upscale models anyway, so this will make it way easier to share chains.


@joeyballentine
There's also another problem that we kinda made ourselves, but have to solve for this to work.

Let's assume we make a path.join node. It simply takes a directory (directory type) and a relative path (string) as its inputs (kinda like Save Image). What should its output type be?

@joeyballentine
Copy link
Member Author

What should its output type be?

a directory, I think

@Athari
Copy link

Athari commented Oct 7, 2022

@RunDevelopment
I like the idea of identifying models with URLs and URNs, however using URIs instead of file paths comes with the extra complexity as it'd be necessary to deal with URL encoding, relative URLs, each URI scheme having its own set of reserved characters etc. I don't know how well this can be handled, especially considering interop between JS and Python.

@RunDevelopment
Copy link
Member

I don't know how well this can be handled, especially considering interop between JS and Python.

As long as we make all paths URLs, there won't any interop issues because both the frontend and the backend will use URLs. Converting between file URLs and paths isn't an issue. NodeJS has 2 handy function for that, so python should also have something like this.

@Athari
Copy link

Athari commented Oct 7, 2022

@RunDevelopment
I'm sure the libraries exist, just please make sure that appending file:../1.jpg to file:///c:/my%20pics/favs/ results in c:\my pics\1.jpg when path appending nodes are implemented.

@RunDevelopment
Copy link
Member

Don't worry, ../1.jpg is a valid (relative) URI, no need for the file: prefix.

@octopushugger
Copy link

Hey, I just want to be able to preserve the directory structure when I upscale a large batch of images in subfolders. Wouldn't it be a very simple change to allow directory to string conversion? I'm confused why directory types aren't just strings in the first place.

And my two cents on chain sharing: if someone wants to make a chain that only works on one platform then why not let them? Cross-platform chain compatibility is great and all but if feature x requires reconfiguration to run properly on other systems that's still a lot better than not having feature x at all.

@joeyballentine
Copy link
Member Author

@octopushugger You can preserve folder structure using the relative path output and input.

@octopushugger
Copy link

@joeyballentine OH, thank you! I tried the relative path input before but until just now my dumbass didn't think about manually specifying the base output folder instead of inputting it from another node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants