Skip to content

Conversation

@jdebacker
Copy link
Member

@jdebacker jdebacker commented Mar 22, 2025

This PR addresses some issues with plot and table functions:

@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2025

Codecov Report

Attention: Patch coverage is 79.16667% with 10 lines in your changes missing coverage. Please review.

Project coverage is 70.17%. Comparing base (e51a130) to head (70c107f).

Files with missing lines Patch % Lines
ogcore/output_plots.py 63.63% 4 Missing ⚠️
ogcore/output_tables.py 69.23% 4 Missing ⚠️
ogcore/utils.py 91.30% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1024      +/-   ##
==========================================
- Coverage   70.26%   70.17%   -0.10%     
==========================================
  Files          20       20              
  Lines        5055     5073      +18     
==========================================
+ Hits         3552     3560       +8     
- Misses       1503     1513      +10     
Flag Coverage Δ
unittests 70.17% <79.16%> (-6.24%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
ogcore/__init__.py 100.00% <100.00%> (ø)
ogcore/utils.py 84.32% <91.30%> (-0.29%) ⬇️
ogcore/output_plots.py 88.26% <63.63%> (-0.73%) ⬇️
ogcore/output_tables.py 95.15% <69.23%> (-1.67%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jdebacker jdebacker marked this pull request as ready for review March 24, 2025 01:40
@jdebacker jdebacker requested review from Copilot and rickecon March 31, 2025 14:07
@jdebacker
Copy link
Member Author

@rickecon This PR is ready for your review.

Copy link
Contributor

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 plotting and table functionalities by renaming several functions for clarity, introducing a more generic unstationarization utility for time series outputs, and updating documentation accordingly.

  • Renames testing and utility functions (e.g., pct_change_unstationarized → unstationarize_vars, tp_output_dump_table → time_series_table).
  • Adjusts handling of stationarized versus unstationarized data in plotting and table outputs.
  • Updates package version and CHANGELOG to reflect the changes.

Reviewed Changes

Copilot reviewed 8 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/test_utils.py Renamed tests to match updated utility functions.
tests/test_output_tables.py Updated test to use the new time_series_table function.
setup.py Bumped version to 0.14.2.
ogcore/utils.py Replaced pct_change_unstationarized with unstationarize_vars and refactored computation.
ogcore/output_tables.py Renamed table function and integrated unstationarization logic.
ogcore/output_plots.py Adjusted default stationarized flag and unstationarizing logic.
ogcore/init.py Updated version to 0.14.2.
CHANGELOG.md Documented functional updates and renaming changes.
Files not reviewed (4)
  • docs/book/content/api/output_plots.rst: Language not supported
  • docs/book/content/api/output_tables.rst: Language not supported
  • docs/book/content/api/parameter_plots.rst: Language not supported
  • docs/book/content/api/utils.rst: Language not supported
Comments suppressed due to low confidence (1)

ogcore/utils.py:1235

  • [nitpick] Consider replacing 'np.arange(params.T)' with 'np.arange(T)' since T is already defined, to improve consistency and clarity.
prod_growth = np.exp(params.g_y * np.arange(params.T))

rickecon and others added 3 commits April 3, 2025 21:09
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@rickecon
Copy link
Member

rickecon commented Apr 4, 2025

@jdebacker. I just submitted a PR to your branch (PR #36) that updates the README.md and the corresponding part of the intro.md in the documentation. Once you review and merge (or not merge) that PR and respond to my suggestion that the default in output_plots.py be stationarized=True, then this is ready to merge.

@rickecon
Copy link
Member

rickecon commented Apr 5, 2025

@jdebacker. I think there is just one more change to fix in line 23 of output_tables.py to set stationarized=True as the default. Once this is done, this PR is ready to merge.

@rickecon
Copy link
Member

rickecon commented Apr 6, 2025

@jdebacker. Great. Merging now.

@rickecon rickecon merged commit b753779 into PSLmodels:master Apr 6, 2025
8 checks passed
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