Adding gen_at_report plus unit tests #2915
Conversation
jeffng-or
force-pushed
the
at-gen_at_report
branch
2 times, most recently
from
4566089 to
d937d5d
Compare
lint fixes one more lint fix moved gen_at_report and associated files from utils to scripts Signed-off-by: Jeff Ng <jeffng@precisioninno.com>
jeffng-or
force-pushed
the
at-gen_at_report
branch
from
d937d5d to
5eb98ac
Compare
|
Tests were dying due to an import issue with utils. I wonder if we should rename either utils.py or the utils directory to avoid confusion. Usually, you don't have a file and a directory with effectively the same name. |
| # To get relative imports to work | ||
| import os, sys | ||
|
|
||
| sys.path.append(os.path.dirname(os.path.realpath(__file__))) |
There was a problem hiding this comment.
Instead of relative imports - why not add in pyproject toml?
https://github.com/The-OpenROAD-Project/OpenROAD-flow-scripts/blob/master/tools/AutoTuner/pyproject.toml
There was a problem hiding this comment.
yeah, I wasn't real happy with the solution either, but was struggling to make it work with the unit tests plus calling from a script.
There was a problem hiding this comment.
@luarss , I remember now. I added the relative imports because I wanted to be able to run the script from within my local workspace. My understanding, which might be wrong, was the pyproject.toml works for when you publish the package, but maybe not within the local workspace. Is my understanding wrong?
There was a problem hiding this comment.
No - it should work as long as you have the Python virtual environment activated. In your case, it should import something like:
To use the scripts:
from autotuner.scripts import moduleA, moduleBFor this init.py, it then can be empty
| ## POSSIBILITY OF SUCH DAMAGE. | ||
| ############################################################################### | ||
|
|
||
| from datetime import datetime |
There was a problem hiding this comment.
Could this be placed in utils/? Any user created libs that we have to import .
Then we can add utils to the [tool.setuptools.packages.find] section.
There was a problem hiding this comment.
yeah, I originally had it in utils, but moved it due to the possible name collision with utils. I'll try re-PR'ing after the dust settles
|
We should probably rename the |
probably a good idea |
Committing gen_at_report.py scripts for extracting information related to AutoTuner runs. Provided to customer as an example.
Includes unit tests which cover 90%+ of the statements