Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warnings as error reporting #94

Merged
merged 12 commits into from
Nov 13, 2024
Merged

Warnings as error reporting #94

merged 12 commits into from
Nov 13, 2024

Conversation

mlxd
Copy link
Member

@mlxd mlxd commented Nov 13, 2024

This PR adds support for a weekly report using updated data of the warnings as errors PR action in PennyLane. A table is added with the determined warnings (not dynamic due to no templated generation of markdown).

The table ie formatted as follows:

PennyLane

UserWarning
DeprecationWarning
pytest.PytestRemovedIn9Warning
PendingDeprecationWarning
pytest.PytestUnraisableExceptionWarning
numpy.exceptions.ComplexWarning
RuntimeWarning
pytest.PytestCollectionWarning
FutureWarning
pennylane.PennyLaneDeprecationWarning

Data is first collected and preprocessed from the latest https://github.com/PennyLaneAI/pennylane/actions/workflows/package_warnings_as_errors.yml run, with the resulting post-processed data created in a temporary tag under this repo. This creation allows us to use the resulting JSON data file in badges to allow dynamic updates of the above.

@mlxd mlxd requested a review from mudit2812 November 13, 2024 19:41
@mlxd mlxd changed the title [WIP] Warnings as error reporting Warnings as error reporting Nov 13, 2024
README.md Show resolved Hide resolved
mlxd and others added 2 commits November 13, 2024 16:08
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
Copy link
Contributor

@PietropaoloFrisoni PietropaoloFrisoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mlxd, this looks great. I have a 'conceptual' question. I see that 2 days ago this PR was merged to test PL with warnings promoted to errors once a week.

However, it seems to me that the amount of failures is so large that it's hard to even read the output log (which should be expected since, in principle, there might be a lot of different warnings).

Was the ultimate purpose of the previous PR to have something like the formatted table that you show in the description of this PR?

@mlxd
Copy link
Member Author

mlxd commented Nov 13, 2024

Thanks @mlxd, this looks great. I have a 'conceptual' question. I see that 2 days ago this PR was merged to test PL with warnings promoted to errors once a week.

However, it seems to me that the amount of failures is so large that it's hard to even read the output log (which should be expected since, in principle, there might be a lot of different warnings).

Was the ultimate purpose of the previous PR to have something like the formatted table that you show in the description of this PR?

Thanks @PietropaoloFrisoni
No, the purpose of the other PR was to have the entire list of complaints --- all CI runs with associated failures that are trackable across the GH API endpoints. Since the PTM is just a reporter, we consume the data here to summarise the above. However, the full source of truth is in the originating PR. The longer term goal is a more expressive set of calls to the GH APIs to extract only the info we need.

For now, polling all the logs and extracted the sections with grep & awk is the fastest path to having the info in one place.

Copy link
Contributor

@PietropaoloFrisoni PietropaoloFrisoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank again!

.github/workflows/scripts/unique_warning_reporter.sh Outdated Show resolved Hide resolved
Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
@mlxd mlxd merged commit 3ecca7d into master Nov 13, 2024
34 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants