search

cms-L1TK / project_generation_scripts

Python scripts to generate the wiring map of the tracklet pattern recognition & the top-level HDL that calls the HLS modules in the Hybrid Chain.
1 stars 2 forks source link

Shebang update #43

Closed meisonlikesicecream closed 2 years ago

meisonlikesicecream commented 2 years ago

Scripts now use python3. Left the shebangs in the archived folders untouched. Also updated the gitignore to ignore the files that are created by the scripts, let me know if there's a reason not to do this :-)

aehart commented 2 years ago

Looks good. My only suggestion is to update this line in the README to indicate that Python 2 is no longer actively supported: https://github.com/cms-L1TK/project_generation_scripts/blob/005b1f27ccbd5e2c608871d524852248cc9efc37/README.md?plain=1#L135

meisonlikesicecream commented 2 years ago

Done!

pwittich commented 2 years ago

wait for python 3 you can now get rid of all the from __future__ import ... stuff -- can you do that too please; that was only needed for py2

pwittich commented 2 years ago

also I just noticed that the pylint workflow only checks a few files, shouldn't we expand that too now?

meisonlikesicecream commented 2 years ago

wait for python 3 you can now get rid of all the from __future__ import ... stuff -- can you do that too please; that was only needed for py2

Done!

meisonlikesicecream commented 2 years ago

also I just noticed that the pylint workflow only checks a few files, shouldn't we expand that too now?

Makes sense, so I added all python files now. Fixed one issue that pylint found, but I'm not sure what to do about the error in WriteVLOGSyntax.py caused by a variable interface not being defined? (https://github.com/cms-L1TK/project_generation_scripts/blob/master/WriteVLOGSyntax.py#L102) It's not a quickfix (I think) so not sure if I should debug it since we're not using that file. I think much of that file is a bit out-dated, so it would need more TLC if we want it to run.

aehart commented 2 years ago

also I just noticed that the pylint workflow only checks a few files, shouldn't we expand that too now?

Makes sense, so I added all python files now. Fixed one issue that pylint found, but I'm not sure what to do about the error in WriteVLOGSyntax.py caused by a variable interface not being defined? (https://github.com/cms-L1TK/project_generation_scripts/blob/master/WriteVLOGSyntax.py#L102) It's not a quickfix (I think) so not sure if I should debug it since we're not using that file. I think much of that file is a bit out-dated, so it would need more TLC if we want it to run.

Yeah I don't think we want to devote any time to WriteVLOGSyntax.py right now. So let's remove it from the pylint checks and maybe it also makes sense to move it to the archive/ directory? If there are any other files not currently in use, the same could apply to them as well.

meisonlikesicecream commented 2 years ago

also I just noticed that the pylint workflow only checks a few files, shouldn't we expand that too now?

Makes sense, so I added all python files now. Fixed one issue that pylint found, but I'm not sure what to do about the error in WriteVLOGSyntax.py caused by a variable interface not being defined? (https://github.com/cms-L1TK/project_generation_scripts/blob/master/WriteVLOGSyntax.py#L102) It's not a quickfix (I think) so not sure if I should debug it since we're not using that file. I think much of that file is a bit out-dated, so it would need more TLC if we want it to run.

Yeah I don't think we want to devote any time to WriteVLOGSyntax.py right now. So let's remove it from the pylint checks and maybe it also makes sense to move it to the archive/ directory? If there are any other files not currently in use, the same could apply to them as well.

Okay I moved quite a lot of unused files to archive now :-)

aehart commented 2 years ago

Thanks! This is actually a lot nicer to navigate without all the artifacts of the past.

I'm happy. Are you happy @pwittich?