Skip to content

HelpersTask597_Configure_Codecov_for_coverage #598

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

Merged
merged 66 commits into from
May 2, 2025

Conversation

Shaunak01
Copy link
Collaborator

Issue: #597

@Shaunak01 Shaunak01 self-assigned this Apr 21, 2025
Copy link

codecov bot commented Apr 21, 2025

Codecov Report

Attention: Patch coverage is 12.50000% with 28 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
helpers/lib_tasks_pytest.py 12.50% 26 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@Shaunak01 Shaunak01 requested review from dremdem and gpsaggese April 22, 2025 18:09
Copy link
Contributor

@dremdem dremdem left a comment

Choose a reason for hiding this comment

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

Good job.
Some comments.

@Shaunak01 Shaunak01 requested a review from dremdem May 1, 2025 01:36
@dremdem
Copy link
Contributor

dremdem commented May 1, 2025

@Shaunak01
Can we run linter?

Copy link
Contributor

@dremdem dremdem left a comment

Choose a reason for hiding this comment

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

LG. Just some nits and comments.

Shaunak01 added 4 commits May 1, 2025 14:43
Pre-commit checks:
- 'check_master' passed
- 'check_author' passed
- 'check_file_size' passed
- 'check_python_compile' passed
- 'check_gitleaks' passed
All checks passed ✅
…b.com:causify-ai/helpers into HelpersTask597_Configure_Codecov_for_coverage

Pre-commit checks:
- 'check_master' passed
- 'check_author' passed
- 'check_file_size' passed
- 'check_python_compile' passed
- 'check_gitleaks' passed
All checks passed ✅
Pre-commit checks:
- 'check_master' passed
- 'check_author' passed
- 'check_file_size' passed
- 'check_python_compile' passed
- 'check_gitleaks' passed
All checks passed ✅
Pre-commit checks:
- 'check_master' passed
- 'check_author' passed
- 'check_file_size' passed
- 'check_python_compile' passed
- 'check_gitleaks' passed
All checks passed ✅
Copy link
Contributor

@gpsaggese gpsaggese left a comment

Choose a reason for hiding this comment

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

LGTM

Can we merge

Pre-commit checks:
- 'check_master' passed
- 'check_author' passed
- 'check_file_size' passed
- 'check_python_compile' passed
- 'check_gitleaks' passed
All checks passed ✅
@causify-ai causify-ai deleted a comment from dremdem May 1, 2025
@causify-ai causify-ai deleted a comment from dremdem May 1, 2025
@Shaunak01
Copy link
Collaborator Author

@dremdem I have run the linter not sure abt the error. The files are linted

################################################################################
* Failed assertion *
cond=False
The Git client is not clean:
dev_scripts_helpers/update_devops_packages/notebooks/Master_buildmeister_dashboard.ipynb
################################################################################

@Shaunak01 Shaunak01 requested a review from dremdem May 1, 2025 18:04
@dremdem
Copy link
Contributor

dremdem commented May 1, 2025

@dremdem I have run the linter not sure abt the error. The files are linted

I ran the Linter on this file locally and got:

RuntimeError: /venv/lib/python3.12/site-packages/nbformat/__init__.py:96: MissingIDFieldWarning: Cell is missing an id field, this will become a hard error in future nbformat versions. You may want to use `normalize()` on your notebooks before validations (available since nbformat 5.1.4). Previous versions of nbformat are fixing this issue transparently, and will stop doing so in the future. [process_jupytext]
Traceback (most recent call last): [process_jupytext]
[jupytext] Reading dev_scripts_helpers/update_devops_packages/notebooks/Master_buildmeister_dashboard.ipynb in format ipynb [process_jupytext]
dev_scripts_helpers/update_devops_packages/notebooks/Master_buildmeister_dashboard.ipynb: Cell content differ on code cell #7: --- expected [process_jupytext]
process_jupytext failed with command `$(find -wholename '*dev_scripts_helpers/notebooks/process_jupytext.py') -f dev_scripts_helpers/update_devops_packages/notebooks/Master_buildmeister_dashboard.ipynb --action test`
 [process_jupytext]

Found some info
https://stackoverflow.com/questions/75632445/anaconda-jupyter-notebook-and-missingidfieldwarning

Tried that approach:

import nbformat

with open("problematic_notebook.ipynb", "r") as file:
    nb_corrupted = nbformat.reader.read(file)

nbformat.validator.validate(nb_corrupted)
# <stdin>:1: MissingIDFieldWarning: Code cell is missing an id field, 
# this will become a hard error in future nbformat versions. 
# You may want to use `normalize()` on your notebooks before validations (available since nbformat 5.1.4). 
# Previous versions of nbformat are fixing this issue transparently, and will stop doing so in the future.

nb_fixed = nbformat.validator.normalize(nb_corrupted)
nbformat.validator.validate(nb_fixed[1])
# Produces no warnings or errors.

with open("fixed_notebook.ipynb", "w") as file:
    nbformat.write(nb_fixed[1], file)

But no luck.

Feel free to file a bug.
FYI @sonniki

Copy link
Contributor

@dremdem dremdem left a comment

Choose a reason for hiding this comment

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

LGTM.
Let’s address a few nits, and feel free to ask Samarth for a review.

@gpsaggese
Copy link
Contributor

Let's merge this and then we do another iteration. We can't have PRs in review for 2 weeks. We want to maintain high tempo

@gpsaggese gpsaggese merged commit 9725e22 into master May 2, 2025
3 of 4 checks passed
@gpsaggese gpsaggese deleted the HelpersTask597_Configure_Codecov_for_coverage branch May 2, 2025 14:47
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