fix pyccel-clean to remove empty directories left behind after a recursive clean#2503
fix pyccel-clean to remove empty directories left behind after a recursive clean#2503triangle-motelti wants to merge 5 commits intopyccel:develfrom
Conversation
|
Hello! Welcome to Pyccel! Thank you very much for your contribution ❤️. I am the GitHub bot. I will help guide you through the different stages necessary to validate a review in Pyccel. If you haven't yet seen our developer docs make sure you check them out here. Amongst other things they describe the review process that we have just started. You can also get in touch with our other developers on our Pyccel Discord Server. To begin with I will give you a short checklist to make sure your pull request is complete. Please tick items off when you have completed them or determined that they are not necessary for this pull request. If you want me to run any specific tests to check out corner cases that you can't easily check on your computer, you can request this using the command Please begin by requesting your checklist using the command |
|
@yguclu, @EmilyBourne, @bauom, please can you check if I can trust this user. If you are happy, let me know with |
|
/bot trust user @triangle-motelti |
|
@triangle-motelti It looks like the Pyccel maintainers trust you 🚀. I will now start running tests when you request them. |
|
@triangle-motelti thank you very much for this PR. This seems like a good idea |
|
@triangle-motelti just to let you know, we are planning to release a new version of Pyccel in the next couple of days (as soon as the last few PRs are merged). If you want to make sure that this PR is included so you have easy access to your fix immediately, please complete the checklist and fix the Codacy error. |
|
I wonder if this behaviour should be triggerable by a flag (or deactivatable)? I can imagine some people building a folder structure and being annoyed if the structure is deleted. |
|
I can't seem to find your checklist to confirm that you have completed all necessary tasks. Please request one using |
|
I fixed the Codacy error |
|
/bot run docs |
|
Here is your checklist. Please tick items off when you have completed them or determined that they are not necessary for this pull request:
|
Your checklist is in your last comment. The bot modifies your message so you have the rights to modify it |
yguclu
left a comment
yguclu
left a comment
There was a problem hiding this comment.
Good job @triangle-motelti! Thanks for this PR
I have a few minor comments
| def remove_pyccel_files(file_name, remove_shared_libs=False, remove_programs=False): | ||
| """Remove files generated by Pyccel based on type and flags""" |
There was a problem hiding this comment.
Could you please expand this docstring and add the Parameters and Returns sections?
| help='The folders to be cleaned (default is the current folder)') | ||
| parser.add_argument('-n', '--not-recursive', action='store_false', | ||
| help='Only run pyccel-clean in the current directory. Do not recurse into other folders') | ||
| help='Only run pyccel-clean in the current directory. Do not recurse into other folders') | ||
| parser.add_argument('-s', '--remove-libs', action='store_true', | ||
| help='Also remove any libraries generated by Python from the folder. Beware this may remove shared libraries generated by tools other than pyccel') | ||
| help='Also remove any libraries generated by Python from the folder. Beware this may remove shared libraries generated by tools other than pyccel') | ||
| parser.add_argument('-p', '--remove-programs', action='store_true', | ||
| help='Also remove any programs from the folder. Beware this may remove programs unrelated to pyccel') | ||
| help='Also remove any programs from the folder. Beware this may remove programs unrelated to pyccel') |
| The folders `__pyccel__X` are called `__pyccel__` unless | ||
| they were generated using `pytest-xdist`. | ||
| def remove_pyccel_folders(file_name): | ||
| """Remove __pyccel__ or __epyccel__ folders""" |
There was a problem hiding this comment.
Could you please expand this docstring and add the Parameters and Returns sections?
| file_name = os.path.join(path_dir, f) | ||
|
|
||
| if remove_pyccel_folders(file_name): | ||
| continue | ||
| elif not os.path.isfile(file_name) and recursive: | ||
| pyccel_clean(file_name, recursive, remove_shared_libs, remove_programs) | ||
| elif f.endswith('.pyccel'): | ||
| os.remove(file_name) | ||
| elif remove_shared_libs and f.endswith(ext_suffix): | ||
| os.remove(file_name) | ||
| elif remove_programs and os.access(file_name, os.X_OK): | ||
| os.remove(file_name) | ||
| if not os.listdir(file_name): | ||
| os.rmdir(file_name) | ||
| elif remove_pyccel_files(file_name, remove_shared_libs, remove_programs): | ||
| continue |
There was a problem hiding this comment.
I think we should check that file_name is really a folder:
| file_name = os.path.join(path_dir, f) | |
| if remove_pyccel_folders(file_name): | |
| continue | |
| elif not os.path.isfile(file_name) and recursive: | |
| pyccel_clean(file_name, recursive, remove_shared_libs, remove_programs) | |
| elif f.endswith('.pyccel'): | |
| os.remove(file_name) | |
| elif remove_shared_libs and f.endswith(ext_suffix): | |
| os.remove(file_name) | |
| elif remove_programs and os.access(file_name, os.X_OK): | |
| os.remove(file_name) | |
| if not os.listdir(file_name): | |
| os.rmdir(file_name) | |
| elif remove_pyccel_files(file_name, remove_shared_libs, remove_programs): | |
| continue | |
| file_name = os.path.join(path_dir, f) | |
| is_file = os.path.isfile(file_name) | |
| is_dir = os.path.isdir(file_name) | |
| if is_dir and remove_pyccel_folders(file_name): | |
| continue | |
| elif is_dir and recursive: | |
| pyccel_clean(file_name, recursive, remove_shared_libs, remove_programs) | |
| if not os.listdir(file_name): | |
| os.rmdir(file_name) | |
| elif is_file and remove_pyccel_files(file_name, remove_shared_libs, remove_programs): | |
| continue |
3 participants
In the
pyccel_clean.pyfile, thepyccel_cleanfunction has a bug that prevents it from cleaning directories recursively as intended.When the function encounters a subdirectory, it correctly calls itself to clean that subdirectory. However, after the recursive call returns, it doesn't do anything with the now-empty subdirectory. An empty directory that previously contained Pyccel-generated files (like
__pyccel__) will be left behind.The
pyccel_cleanfunction should check if a subdirectory is empty after a recursive call. if it is, the function should remove it.This change ensures that after recursively cleaning a directory, the function checks if the directory has become empty and, if so, removes it. This will result in a much cleaner directory tree after running the command.