fix: quiet default logging for notebook/API use; sharpen actionable notices - #40
Open
micahpw wants to merge 1 commit into
Open
fix: quiet default logging for notebook/API use; sharpen actionable notices#40micahpw wants to merge 1 commit into
micahpw wants to merge 1 commit into
Conversation
…ty (#22) Closes the remaining part of #22: a plain `import gat` inherited loguru's untouched default sink, which shows every DEBUG-level call across the codebase (~90 call sites), not just the plot-function registration noise originally reported. - gat/__init__.py: on first import, if loguru is still at its pristine single-handler state, set the same quiet default the CLI already uses for itself (WARNING). Never touches an already- configured sink -- verified a caller's own logger.add()/remove() survives `import gat` untouched, and CLI's own setup_cli_logging() still overrides normally. warnings.warn(...), the channel GAT uses for actionable notices, is untouched either way. - models/scenario.py: the "no fuzzy match, assigning random color" warning didn't explain how to fix it, unlike its sibling fuzzy-match warning -- now it does (technology_mappings config override). - scenariohandlers/multi.py: MultiScenario._concat_gat_df silently unioned differing scenario timestamp indexes via pd.concat(axis=1), introducing NaN rows with no signal when scenarios cover different date ranges/resolutions. Now warns with each scenario's actual range. Verified with a fresh interpreter/subprocess, not just reasoning: DEBUG spam gone, WARNING-level still visible, a pre-existing sink survives import, and the two new warnings fire/don't fire on synthetic misaligned/aligned cases respectively.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the remaining part of #22 (the "unsolicited DEBUG log spam on plain import" section) plus two related notebook-usability gaps surfaced while scoping it.
Background
#22already got most of its fixes earlier (lazyscenariohandlersimports, dropping thequickplots/matplotlib import fromplexos.py, de-blanketeddatahelpers). The one remaining piece: a plainimport gatinherits loguru's untouched default sink, which -- verified, not assumed -- shows everyDEBUG-level call across the codebase (~90 call sites), not just the plot-function registration spam originally reported.Changes
src/gat/__init__.py-- on first import, if loguru is still at its pristine single-handler state (len(handlers) == 1 and 0 in handlers), set the same quiet default the CLI already uses for itself (level="WARNING", mirroringsetup_cli_logging). Never touches an already-configured sink.warnings.warn(...)-- the channel GAT already uses for actionable notices -- is untouched either way.src/gat/models/scenario.py-- the "no fuzzy match, assigning random color" warning didn't explain how to fix it, unlike its sibling fuzzy-match warning right above it. Now it does (points at thetechnology_mappingsconfig override).src/gat/scenariohandlers/multi.py--MultiScenario._concat_gat_dfsilently unioned differing scenario timestamp indexes viapd.concat(axis=1), introducing NaN rows with zero signal when scenarios cover different date ranges/resolutions. Now warns, naming each scenario's actual range.Why not just
logger.disable("gat")/logger.remove()Both considered and rejected:
logger.remove()at import is destructive -- loguru'sloggeris a single global singleton per process, so it would silently wipe out a sink a notebook user (or another library) already configured before importinggat. Reproduced this directly.logger.disable("gat")would also silence the ~90logger.warning/logger.errorcalls throughoutloader.py,simulations/,backends/,discovery.py-- many carry real operational signal (parse failures, fallback behavior), not CLI-only noise. Blanket-disabling those seemed like the wrong tradeoff.Verification
Ran in subprocesses / fresh interpreters, not just reasoned about:
import gat; import gat.quickplots.dispatch-- noDEBUGin output (was 3 lines before)logger.warning(...)afterimport gat-- still visibleimport gat-- still receives messages aftersetup_cli_logging()-- still overrides normallywarnings.warn(...)-- unaffected either wayMultiScenariowith offset date ranges -- warns with both scenarios' actual ranges; matching ranges -- silent;get_generation_capacity(not time-series) -- never checkedNew tests:
tests/test_logging_defaults.py(subprocess-based, since the behavior only fires on a package's first import),tests/handlers/test_multiscenario_time_alignment.py. Existing test updated:tests/models/test_data_models.py(asserted on the old warning string).Full suite: 241 passed, 57 skipped, 0 failed (excluding
test_plexos_regression.py, which fails identically with or without this change -- confirmed pre-existing, a local fixture-data mismatch unrelated to this PR).🤖 Generated with Claude Code