Refactor to use pathlib #210

csatt · 2023-06-11T00:31:27Z

This Issue was brought to my attention by the "refurb" tool. Among many others, refurb makes the following suggestions:

[FURB104]: Replace os.getcwd() with Path.cwd()
[FURB141]: Replace os.path.exists(x) with Path(x).exists()
[FURB144]: Replace os.remove(x) with Path(x).unlink()
[FURB150]: Replace os.makedirs(x) with Path(x).mkdir(parents=True)

"Path" is referring to the Path class of the pathlib module. The pathlib module was introduced in Python 3.4, and is a more "modern" way to deal with the filesystem using objects instead of strings. There are many articles about why new code should use pathlib instead of the os module, and many advocate refactoring existing code. There's a pretty good counterargument, which is, "if it ain't broke, don't fix it." But I have generally tended to try to incrementally improve the IV Swinger 2 code toward adhering to Python coding best practices, where feasible.

The four cases that refurb calls out are just scratching the surface. There are many other cases of os module functions that can and should be refactored to use pathlib. This Issue will track these changes.

I will file another Issue later to address all of the other refurb suggestions not related to pathlib.

The text was updated successfully, but these errors were encountered:

Decided that the uses of os.getcwd() in IV_Swinger2.py and IV_Swinger2_gui.py should be removed. The current directory really cannot be the run directory. Converted the two instances in the main() function of IV_Swinger2_PV_model.py.

This is an incremental change. For now, all instances of os.path.exists(x) are replaced with Path(x).exists(). Later, all the "x" paths will themselves be changed from strings to Path objects, and these will all be changed from Path(x).exists(x) to simply x.exists().

This is an incremental change. For now, all instances of os.remove(x) are replaced with Path(x).unlink(). Later, all the "x" paths will themselves be changed from strings to Path objects, and these will all be changed from Path(x).unlink(x) to simply x.unlink().

This is an incremental change. For now, all instances of os.makedirs(x) are replaced with Path(x).mkdir(parents=True) Later, all the "x" paths will themselves be changed from strings to Path objects, and these will all be changed from x.mkdir(parents=True)

This is one of many incremental changes to refactor the code to use pathlib Path objects. In this commit, the app_data_dir path is converted, as well as paths that have app_data_dir immediately on the RHS of their assignment. It only goes that far, though. This commit is to the pathlib_refactor branch and will be merged to the default branch after all the other changes have been committed and thorough testing has been performed.

This is another incremental change to refactor the code to use pathlib Path objects. In this commit, the hdd_output_dir path is converted, as well as paths that have it immediately on the RHS of their assignment. There are a few instances where this is propagated a bit further, but for the most part that is as far as it goes.. The commit also includes a change to remove the gen_dbg_str global function from IV_Swinger2.py. It is superseded by the print_dbg_str global function in IV_Swinger.py. That function is aliased to PRINT_DBG_STR in IV_Swinger2.py, IV_Swinger2_gui.py, and IV_Swinger2_sim.py (so far). This commit is to the pathlib_refactor branch and will be merged to the default branch after all the other changes have been committed and thorough testing has been performed.

This is another incremental change to refactor the code to use pathlib Path objects. The following are converted: sensor_info_filename run_info_filename cfg_filename starting_cfg_filename copy_dir current_img run_dir cfg_file config_dir overlay_dir gif_full_path This commit is to the pathlib_refactor branch and will be merged to the default branch after all the other changes have been committed and thorough testing has been performed.

I am no longer going to attempt to say what my reason is for grouping the changes in each of these incremental commits. I'm just making changes that can be made until I feel like it's time to do a commit.

Also temporarily import "icecream" which is probably a better tool than the print_dbg_str function that I added a few commits ago.

Various

This one covers IV_Swinger2_PV_model.py

This one converts the last few os.path usages in IV_Swinger_plotter.py to pathlib equivalents. It also adds the --plot_dir option to the plotter, which allows the removal of the os.chdir() call in the run() method of the IV_Swinger2_plotter class in IV_Swinger2.py.

This one mostly converts calls to open() to calls to the open() method or the Path object. However, some are replaced by calls to the read_text(), write_text(), or touch() methods.

Changed from glob.glob() to Path().glob() Protected a few comparisions between Path() objects from generating false positives/negatives if one is actually a string by adding .resolve(). That would generate an exception if it's a string. Replaced call to os.access() with a try/except.

This one is related to the previous commit in the sense that the changes are related to file access. It's not really specific to pathlib, however. The biggest change is to remove most of the exception handling for file access from IV_Swinger2.py (and IV_Swinger_plotter.py.) Most such exceptions would just print and log an error message. But for anyone running the GUI, that wouldn't be very noticeable. Instead, it's better to just let the exceptions propagate up to the GUI and either handle them explicitly there, or let the catch-all exception handler generate the message with the exception text. In IV_Swinger2_gui.py, the exceptions that are explicitly caught look for either PermissionError if that is specifically what is being checked for, or OSError, which is more general. IOError was merged into OSError in Python 3.3.

csatt added the cosmetic Code cleanup, etc. label Jun 11, 2023

csatt self-assigned this Jun 11, 2023

csatt added a commit that referenced this issue Jul 6, 2023

Next incremental batch of pathlib changes (Issue #210)

51bd17d

Also temporarily import "icecream" which is probably a better tool than the print_dbg_str function that I added a few commits ago.

csatt added a commit that referenced this issue Jul 6, 2023

Next incremental batch of pathlib changes (Issue #210)

3e4e093

Various

csatt added a commit that referenced this issue Jul 7, 2023

Next incremental batch of pathlib changes (Issue #210)

55480d5

Various

csatt added a commit that referenced this issue Jul 7, 2023

Next incremental batch of pathlib changes (Issue #210)

c6003a9

This one covers IV_Swinger2_PV_model.py

csatt mentioned this issue Jul 21, 2023

Remove IVS1-only code #213

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor to use pathlib #210

Refactor to use pathlib #210

csatt commented Jun 11, 2023

Refactor to use pathlib #210

Refactor to use pathlib #210

Comments

csatt commented Jun 11, 2023