Skip to content

Specify stacklevel in calls to warnings.warn#327

Closed
kandersolar wants to merge 2 commits intodevelopmentfrom
stacklevel
Closed

Specify stacklevel in calls to warnings.warn#327
kandersolar wants to merge 2 commits intodevelopmentfrom
stacklevel

Conversation

@kandersolar
Copy link
Member

@kandersolar kandersolar commented Apr 29, 2022

  • Code changes are covered by tests
  • Code changes have been evaluated for compatibility/integration with TrendAnalysis
  • New functions added to __init__.py
  • API.rst is up to date, along with other sphinx docs pages
  • Example notebooks are rerun and differences in results scrutinized
  • Updated changelog

This is a change I've wanted to make for a while: specify a better stacklevel when emitting warnings. The stacklevel controls which line of code is shown as causing the warning, with the default (stacklevel=1) being the call to warnings.warn itself. Increasing the stacklevel usually results in a more useful line of code being shown. For example, our current example notebooks show warnings like this:

/Users/mdecegli/opt/anaconda3/envs/xgboost/lib/python3.7/site-packages/rdtools/soiling.py:15: UserWarning: The soiling module is currently experimental. The API, results, and default behaviors may change in future releases (including MINOR and PATCH releases) as the code matures.
  'The soiling module is currently experimental. The API, results, '

The source code line shown below the warning is not really useful to the user. In this case it's just the string literal containing the warning message; other times it's something like warnings.warn(.... What would be more useful is to show one level up in the call stack (stacklevel=2), which would look like this:

c:\users\kanderso\software\anaconda3\envs\rdtools-dev\lib\site-packages\rdtools\analysis_chains.py:582: UserWarning: The soiling module is currently experimental. The API, results, and default behaviors may change in future releases (including MINOR and PATCH releases) as the code matures.
  from rdtools import soiling

The best improvements are when the relevant higher-level call is a single line of code. Multi-line expressions are not always handled very well, for example:

c:\users\kanderso\software\anaconda3\envs\rdtools-dev\lib\site-packages\rdtools\analysis_chains.py:850: UserWarning: The soiling module is currently experimental. The API, results, and default behaviors may change in future releases (including MINOR and PATCH releases) as the code matures.
  results_dict['calc_info'], **kwargs)

Another complication is that the relevant stack level might be different depending how the function is called. For example logic_clip_filter can be called directly (stacklevel=2) or indirectly via clip_filter(..., model='logic') (stacklevel=3).

Regardless, specifying stacklevel doesn't make things any worse, so I don't see a reason not to use them. This IPython blog post has a bit of interesting commentary on this topic.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the user experience by adding stacklevel parameters to all warnings.warn() calls throughout the RDTools codebase. This causes warnings to display the line of code that triggered them (from the user's perspective) rather than internal implementation details, making debugging and troubleshooting easier for users.

Changes:

  • Added stacklevel=2 to warnings in public API functions across multiple modules
  • Added stacklevel=3 to warnings in internal helper functions that are called by public APIs
  • Covers 21 warning locations across 7 files

Reviewed changes

Copilot reviewed 7 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
rdtools/soiling.py Added stacklevel to 4 warnings (module import warning, parameter validation, and data quality warnings)
rdtools/plotting.py Added stacklevel=2 to 4 experimental module warnings in plotting functions
rdtools/normalization.py Added stacklevel=3 to data exclusion warning in internal interpolation helper
rdtools/filtering.py Added stacklevel to 5 warnings in clipping filter functions (deprecation, data validation, and experimental API warnings)
rdtools/clearsky_temperature.py Added stacklevel=2 to 2 warnings about input data quality
rdtools/availability.py Added stacklevel to 2 warnings (module import warning and data quality warning)
rdtools/analysis_chains.py Added stacklevel=3 to 3 warnings in internal TrendAnalysis helper methods

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@martin-springer
Copy link
Collaborator

Closing - replaced with #476

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.

2 participants