Skip to content

Reduce the amount of logging messages that get too frequently printed during debug#1721

Merged
amarkpayne merged 2 commits into
masterfrom
less_debug
Sep 27, 2019
Merged

Reduce the amount of logging messages that get too frequently printed during debug#1721
amarkpayne merged 2 commits into
masterfrom
less_debug

Conversation

@alongd

@alongd alongd commented Sep 13, 2019

Copy link
Copy Markdown
Member

Motivation or Problem

When debugging RMG or a software that depends on RMG the amount of debugging statements that get printed jam the buffer, not allowing the developer to conveniently see the actual original error massage.

Description of Changes

Some logging statements, most of which are debugging level, were commented out. I only marked those which get printed too frequently in my experience. The purpose here was to flag them in the code for a discussion of how to best deal with them - leave commented out (so can be conveniently brought back to life if needed) or completely deleting them.
I think we can start the discussion even though we won't merge until the Py3 transition, since the same logging statements will be relevant after transitioning.

@alongd
alongd requested a review from amarkpayne September 13, 2019 17:36
@alongd alongd self-assigned this Sep 13, 2019

@amarkpayne amarkpayne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I talked with @mliu49 and we both agree that we can just get rid of this code instead of commenting it out. There are a couple of changes that need to be made due to the failing unit tests (at least one of them needs to be modified because it expects to see a logging statement that we are now getting rid of.

@alongd
alongd force-pushed the less_debug branch 2 times, most recently from f546992 to d123f68 Compare September 27, 2019 14:12
@codecov

codecov Bot commented Sep 27, 2019

Copy link
Copy Markdown

Codecov Report

Merging #1721 into master will decrease coverage by 0.02%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1721      +/-   ##
==========================================
- Coverage    32.6%   32.57%   -0.03%     
==========================================
  Files          87       87              
  Lines       26115    26090      -25     
  Branches     6874     6866       -8     
==========================================
- Hits         8514     8500      -14     
+ Misses      16640    16616      -24     
- Partials      961      974      +13
Impacted Files Coverage Δ
rmgpy/data/kinetics/family.py 48.28% <ø> (-0.19%) ⬇️
rmgpy/data/kinetics/groups.py 18.01% <ø> (+0.72%) ⬆️
rmgpy/data/kinetics/common.py 70.05% <ø> (-0.16%) ⬇️
rmgpy/data/thermo.py 60.67% <ø> (-0.04%) ⬇️
rmgpy/constraints.py 94.33% <0%> (ø) ⬆️
rmgpy/data/base.py 48.37% <100%> (-0.14%) ⬇️
rmgpy/molecule/adjlist.py 73.16% <0%> (-0.32%) ⬇️
rmgpy/data/statmech.py 42.2% <0%> (ø) ⬆️
rmgpy/rmg/pdep.py 12.21% <0%> (ø) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91cdd1d...55ec216. Read the comment docs.

@alongd

alongd commented Sep 27, 2019

Copy link
Copy Markdown
Member Author

@amarkpayne, this PR should be ready for review now

@amarkpayne amarkpayne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This all looks good, thanks for making these changes!

@amarkpayne
amarkpayne merged commit 3e37656 into master Sep 27, 2019
@amarkpayne
amarkpayne deleted the less_debug branch September 27, 2019 16:36
@mliu49 mliu49 mentioned this pull request Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants