byuccl / bfasst

Tools for FPGA Assurance Flows
Apache License 2.0
12 stars 5 forks source link

create legacy tools #401

Closed yonnorc42 closed 9 months ago

yonnorc42 commented 10 months ago
yonnorc42 commented 10 months ago

@KeenanRileyFaulkner When I run pylint on this branch, I get this error:

(.venv) yonnorc42@CCL2:~/bfasst$ make pylint

find ./scripts ./bfasst -iname "*.py" -exec black -q -l 100 {} \;
git fetch
pylint --errors-only $(git ls-files --directory scripts --directory bfasst | grep -e ".py$")
pylint $(git diff --name-only $(git merge-base origin/main HEAD) | grep -e ".py$")
************* Module bfasst/impl/__init__.py
bfasst/impl/__init__.py:1:0: F0001: No module named bfasst/impl/__init__.py (fatal)
************* Module bfasst/opt/__init__.py
bfasst/opt/__init__.py:1:0: F0001: No module named bfasst/opt/__init__.py (fatal)
************* Module bfasst/reverse_bit/__init__.py
bfasst/reverse_bit/__init__.py:1:0: F0001: No module named bfasst/reverse_bit/__init__.py (fatal)
************* Module bfasst/synth/__init__.py
bfasst/synth/__init__.py:1:0: F0001: No module named bfasst/synth/__init__.py (fatal)

------------------------------------------------------------------
Your code has been rated at 0.00/10 (previous run: 0.00/10, +0.00)

The paths for those files has changed but pylint is still checking their old paths, which is causing the error, but I don't know why.

KeenanRileyFaulkner commented 10 months ago

When you run pylint locally it compares any files you changed. Try running it again locally and see if the second time it gets the files from the correct location? Your pylint checks passed on this PR

yonnorc42 commented 10 months ago

It gets them from the wrong location still no matter how many times it's run.

KeenanRileyFaulkner commented 10 months ago

Give me a minute. I'm looking at your branch and trying to fix it

KeenanRileyFaulkner commented 10 months ago

This may be normal behavior, since you removed files. The pylint make target checks out the common ancestor of your current HEAD and the main branch and compares any files that are different between them with the pylint tool. In your case, you branched off the fix_weeklytests branch, but that's irrelevant. Even if you branched off main, pylint would still try to check files that are missing from your branch. However, the PR is passing those same checks on CI. I'm not 100% clear on why CI doesn't act exactly the same, but I think that once this merges and you pull again, it should go away locally.

KeenanRileyFaulkner commented 10 months ago

Looking at the pylint workflow in the .github/ directory, it looks like CI checks all the python files in your bfasst and scripts directories, but doesn't compare them to main, hence why it passes CI the make pylint target fails. This could be an issue worth opening, so that the make pylint target does the same thing as CI

jgoeders commented 10 months ago

@yonnorc42 @KeenanRileyFaulkner Yeah I run into this a lot and just ignore it when I know it's not an issue. If there's a way to fix this then I'm all for it!

yonnorc42 commented 9 months ago

@jgoeders I think this should be ready for merge