google / globalfoundries-pdk-libs-gf180mcu_fd_pr

Primitives for GF180MCU provided by GlobalFoundries.
https://gf180mcu-pdk.rtfd.io
Apache License 2.0
45 stars 27 forks source link

Shouldn't use `os.system` in Python scripts #21

Closed mithro closed 1 year ago

mithro commented 2 years ago

Expected Behavior

You shouldn't use os.system in Python scripts.

Examples;

Should use python internal functionality;

https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/2e2eb4ad16e9f26ae80f7b0cad70dfd9a10da177/rules/klayout/lvs/testing/run_sc_regression.py#L105

https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/2e2eb4ad16e9f26ae80f7b0cad70dfd9a10da177/rules/klayout/lvs/testing/run_sc_regression.py#L139

Should use subprocess module;

https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/2e2eb4ad16e9f26ae80f7b0cad70dfd9a10da177/rules/klayout/lvs/testing/run_sc_regression.py#L138

Actual Behavior

You should use inbuilt Python functions or the subprocess module like check_call or check_output.

Instead of using rm you should use something like Path.unlink or os.unlink or shutil.rmtree.

mithro commented 2 years ago

https://github.com/google/globalfoundries-pdk-libs-gf180mcu_fd_pr/blob/2e2eb4ad16e9f26ae80f7b0cad70dfd9a10da177/rules/klayout/lvs/testing/run_sc_regression.py#L226

Should probably use https://docs.python.org/3/library/shutil.html?highlight=shutil#shutil.move or https://docs.python.org/3/library/os.html#os.rename or https://docs.python.org/3/library/os.html#os.replace

atorkmabrains commented 2 years ago

@mohanad0mohamed Could you please take care of this one soon?

FaragElsayed2 commented 1 year ago

@atorkmabrains This issue has been addressed in PV repo

atorkmabrains commented 1 year ago

This has been addressed in PV repo as highlighted by @FaragElsayed2

@mithro I'll close this issue. Please don't hesitate to open if you have any other concerns.