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

g.tempfile: allow creation of temporary directories #4397

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ninsbl
Copy link
Member

@ninsbl ninsbl commented Sep 27, 2024

The Python library has a function to create temporary directories: https://grass.osgeo.org/grass84/manuals/libpython/_modules/script/core.html#tempdir

The underlying C-module however does not support that. This PR allows the g.tempfile module to create a temporary directory instead of a temporary file. The functionality is equal to the function in the Python library and the python library is updated to use the new f-flag in g.tempfile.

@ninsbl ninsbl added enhancement New feature or request C Related code is in C general labels Sep 27, 2024
@ninsbl ninsbl added this to the 8.5.0 milestone Sep 27, 2024
@github-actions github-actions bot added Python Related code is in Python HTML Related code is in HTML libraries module docs labels Sep 27, 2024
general/g.tempfile/main.c Outdated Show resolved Hide resolved
@@ -52,6 +53,13 @@ int main(int argc, char *argv[])
dry_run->description =
_("Dry run - don't create a file, just prints it's file name");

directory = G_define_flag();
directory->key = 'f';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d is unfortunately used, but f is not great: folder, file, or directory?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt the same but had no better idea. I considered -D but that did not seem better than -f for folder mode...
Any suggestions?

general/g.tempfile/g.tempfile.html Outdated Show resolved Hide resolved
general/g.tempfile/g.tempfile.html Show resolved Hide resolved
@ninsbl ninsbl requested a review from wenzeslaus October 3, 2024 21:50
@ninsbl
Copy link
Member Author

ninsbl commented Oct 24, 2024

Are the requested changes sufficiently addressed? Any suggestions for a different flag-key? I had and have unfortunately no better idea than -f (or -D, in case)...

Would be happy to get this merged...

@wenzeslaus
Copy link
Member

Any suggestions for a different flag-key? I had and have unfortunately no better idea than -f (or -D, in case)...

I think anything will be better than -f. If the main use of this interface is shell scripting, -f to mean directory/folder as opposed to file is highly unexpected. From letters which come to my mind and don't have other possible meaning here, I suggest u and m. No particular reason for those. An option like type=[file|directory|folder] may work, but in this context where shell is the main usage and Python has a wrapper, one-letter flag seems the right choice.

Comment on lines +58 to +60
If the size of temporary files is not an issue, it is recommended
to use <i>NamedTemporaryFile</i> with a context manager to create a
temporary file in Python.<br>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, you can pass dir to both NamedTemporaryFile and TemporaryDirectory, so you can (could) use the 100% way with the project-based tmp dir if you get the project-mapset-hostname path in some way even without creating subdirectories there.

It is designed for shell scripts that need to use large temporary files.
GRASS provides a mechanism for temporary files or directories that does not
depend on /tmp. GRASS temporary files and directories are created in the
GRASS GIS database with the assumption that there will be enough space for
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can consider talking only about the project. See #3121 Phase 3, bullet point De-emphasize GISDBASE as its own thing from user perspective.

Suggested change
GRASS GIS database with the assumption that there will be enough space for
GRASS GIS project with the assumption that there will be enough space for

You anyway specify only $PROJECT below.

GRASS provides a mechanism for temporary files or directories that does not
depend on /tmp. GRASS temporary files and directories are created in the
GRASS GIS database with the assumption that there will be enough space for
large files. The base directory is: $PROJECT/$MAPSET/.tmp/$HOSTNAME/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it is clear to the reader what host name is. Elsewhere we talk about node or node name. Hard to say, maybe it's fine as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C docs enhancement New feature or request general HTML Related code is in HTML libraries module Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants