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
fs.copy.copy_*_if #460
fs.copy.copy_*_if #460
Conversation
The free functions copy_file, copy_file_if_newer, copy_dir, copy_dir_if_newer, copy_fs, and copy_fs_if_newer now are all implemented in terms of copy_file_if, copy_dir_if, and copy_fs_if. Unit Tests for this change will be created in a future commit. This change streamlined the code by removing duplication in logic and clustering similar logic closer together. Also, though this has not been tested, copy_dir_if_newer should be faster in this implementation when copying to a dst_dir that contains a lot of files that are not present in the respective src_dir. Finally, a bug was fixed that was caused by a false lookup in the dictionary of file infos from dst_fs. This fix could cause unnecessary copies and therefore a decrease in performance.
…s.copy. tests/test_copy experienced a general rework in many areas. Before, tests focused on the most basic of test cases and thus missed more complex situations that could (and actually did) cause errors. Thus, some tests for the same unit were merged to create more relevant test cases.
Imho, |
Nice work! I'll wait for your unit tests to be added. I'm thinking we should soft-deprecate the |
@althonos Thanks. I already added unit tests, I just forgot to remove that part from the description, sorry. It looks like the lines that are not covered anymore are the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small fixes related to the deprecation of older functions, and we should be good to go!
Thank you! First PR out of the way 😃 |
Type of changes
Checklist
Description
Fixes #458
Added _copy_if functions to fs.copy module.
The free functions copy_file, copy_file_if_newer, copy_dir, copy_dir_if_newer, copy_fs, and copy_fs_if_newer now are all implemented in terms of copy_file_if, copy_dir_if, and copy_fs_if.
This change streamlined the code by removing duplication in logic and clustering similar logic closer together.
Also, though this has not been tested, copy_dir_if_newer should be faster in this implementation when copying to a dst_dir that contains a lot of files that are not present in the respective src_dir.
Finally, a bug was fixed that was caused by a false lookup in the dictionary of file infos from dst_fs. This fix could cause unnecessary copies and therefore a decrease in performance.