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

New feature implementation/#2189 Add support for uploading nested folders #2206

Open
wants to merge 25 commits into
base: develop
Choose a base branch
from

Conversation

JosuaCarl
Copy link

#2189

Implementation:

Limitations:

Help needed:

  • Resolve the second limitation (although I don't see how)

@@ -1227,6 +1227,12 @@
"default_value": "False",
"doc": "If set to True, multiple files can be uploaded."
},
{
"name": "webkitdirectory",
Copy link
Member

@FredLL-Avaiga FredLL-Avaiga Nov 7, 2024

Choose a reason for hiding this comment

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

bad name ... who cares about webkit in python ?
allow_folder_selection that would translate to allowFolderSelection ?
What does our naming expert @FabienLelaquais says ?

Copy link
Author

@JosuaCarl JosuaCarl Nov 7, 2024

Choose a reason for hiding this comment

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

I would suggest to get the word nested, like nested_folder_upload in there, as it captures what the code does. It not only uploads the content of folder, but imitates the structure of the folder in the directory it is uploaded to, like:

/tmp
  | -- file1
  | -- dir2
       |-- file1
       |-- file2

Copy link
Member

Choose a reason for hiding this comment

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

that's a nice added benefit but I think the main feature is to be able to select a folder and everything it contains, no ?

Copy link
Author

Choose a reason for hiding this comment

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

Not for me, because I need the structure to be preserved for a project, but I can see, why this is mostly the usecase.

Copy link
Member

Choose a reason for hiding this comment

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

bad name ... who cares about webkit in python ? allow_folder_selection that would translate to allowFolderSelection ? What does our naming expert @FabienLelaquais says ?

What about files_only, defaults to True?

Copy link
Author

Choose a reason for hiding this comment

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

That would solve it the other way round. In my eyes, it sounds a bit like files_only=false would allow the selection of files and folders (which is not the case). Im currently working with selectFolder = false, but I don't have a strong preference.

Copy link
Author

Choose a reason for hiding this comment

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

Another suggestion from my side: I just lost hours to debugging and it was the naming conventions. I.e. I didn't snake_case my variable name for python scripts, because I thought, that I would do that later to aid me connect the variables in my head during repeated changes. I did not realize until now that the snake_case is necessary for the variable to be adequately converted into the right typescript camelCase. I think this could at least be mentioned in the CONTRIBUTING.md. I have done so, if this is not desired, please tell me. Has nobody else had this problem before during playing around with the Gui scripts ?

taipy/gui/gui.py Fixed Show fixed Hide fixed
taipy/gui/gui.py Outdated
# TODO: Add support for directory creation of nested structures
if path:
upload_path = Path(os.path.join( self._get_config("upload_folder", tempfile.gettempdir()), os.path.dirname(path))).resolve()
os.makedirs( upload_path, exist_ok=True )

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
This path depends on a
user-provided value
.
taipy/gui/gui.py Outdated
Comment on lines 1014 to 1016
upload_path = Path(os.path.join( self._get_config("upload_folder",
tempfile.gettempdir()),
os.path.dirname(path))).resolve()

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
This path depends on a
user-provided value
.
@FredLL-Avaiga FredLL-Avaiga added 🖰 GUI Related to GUI 🟨 Priority: Medium Not blocking but should be addressed ✨New feature labels Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🖰 GUI Related to GUI ✨New feature 🟨 Priority: Medium Not blocking but should be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants