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

Python Logger/C++ Debug Mechanism Addition and Journal removal #6

Merged
merged 21 commits into from
Nov 19, 2024

Conversation

mpatrou
Copy link
Collaborator

@mpatrou mpatrou commented Nov 5, 2024

Short description of the changes:

Python logger mechanism added, journal removed and other cleanup changes

Long description of the changes:

The code include the following changes:

  • python logging initialized and added in src/ and pyproject; journal removed
  • python logging initialized and added in tests/; journal removed
  • logging functions that call journal removed in tests/
  • journal removed from lib/; regular print statement inside the DEBUG blocks added
  • all building files for python (pyproject, meta etc.) and C (cmake files) are updated to remove journal
  • environment and pyproject updated to match dev multiphonon dependency versions
  • _version import from init; hard-coded value removed
  • code cleanup

The PR enables python logging with various levels:
pytest --log-cli-level=DEBUG

also the building process of C libraries and python are found and attached in the testing section.

Check list for the pull request

  • I have read the [CONTRIBUTING]
  • I have read the [CODE_OF_CONDUCT]
  • I have added tests for my changes
  • I have updated the documentation accordingly

Check list for the reviewer

  • I have read the [CONTRIBUTING]
  • I have verified the proposed changes
  • best software practices
    • all internal functions have an underbar, as is python standard
    • clearly named variables (better to be verbose in variable names)
    • code comments explaining the intent of code blocks
  • All the tests are passing
  • The documentation is up to date
  • code comments added when explaining intent

Manual test for the reviewer

python part only:

  • pip install -e .
  • pytest

cmake installation python and /C:

  • mkdir build && cd build/
  • export PREFIX=$CONDA_PREFIX
  • cmake -DCONDA_BUILD=TRUE -DCMAKE_INSTALL_PREFIX=$PREFIX -DDEPLOYMENT_PREFIX=$PREFIX ..
  • make
  • make install
  • cd .. && pytest

Print various logging levels in pytest:

  • pytest --log-cli-level=DEBUG
  • pytest --log-cli-level=INFO
  • etc

References

[Story] [Histogram] Replace Journal with python logging module

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2024

Codecov Report

Attention: Patch coverage is 78.04878% with 9 lines in your changes missing coverage. Please review.

Project coverage is 76.96%. Comparing base (7a74f35) to head (9f50543).

Files with missing lines Patch % Lines
src/histogram/ndarray/NumpyNdArray.py 33.33% 4 Missing ⚠️
src/histogram/NdArrayDataset.py 60.00% 2 Missing ⚠️
src/histogram/hpickle.py 0.00% 2 Missing ⚠️
src/histogram/hdf/Renderer.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             next       #6      +/-   ##
==========================================
- Coverage   76.97%   76.96%   -0.02%     
==========================================
  Files          42       42              
  Lines        3175     3173       -2     
==========================================
- Hits         2444     2442       -2     
  Misses        731      731              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@mpatrou mpatrou changed the title Python Logger/C++ Debug Addition and Journal removal Python Logger/C++ Debug Mechanism Addition and Journal removal Nov 18, 2024
@KyleQianliMa KyleQianliMa self-requested a review November 18, 2024 18:46
@KyleQianliMa
Copy link
Collaborator

Tested locally both python and c++ part. Both works with new python logging. Please make sure to squash and merge.

@mpatrou mpatrou merged commit ef929b9 into next Nov 19, 2024
3 checks passed
@mpatrou mpatrou deleted the journal_removal branch November 19, 2024 16:01
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