Skip to content

Tidy up process input code + make input/input commodities => "other" commodities#629

Merged
alexdewar merged 11 commits intomainfrom
other-commodities
Jun 17, 2025
Merged

Tidy up process input code + make input/input commodities => "other" commodities#629
alexdewar merged 11 commits intomainfrom
other-commodities

Conversation

@alexdewar
Copy link
Copy Markdown
Collaborator

Description

I started making the change from input/output commodities to "other" commodities, but realised some of the input code for processes could be tidied up, so did that while I was at it. Hope it's not too annoying that I've mixed these things in one PR.

Cleanups:

  • Change various functions that were taking both maps of processes and process IDs as arguments to just take the maps
  • Refactor tests to check error messages

Other:

Closes #603. Closes #560. Closes #627.

Type of change

  • Bug fix (non-breaking change to fix an issue)
  • New feature (non-breaking change to add functionality)
  • Refactoring (non-breaking, non-functional change to improve maintainability)
  • Optimization (non-breaking change to speed up the code)
  • Breaking change (whatever its nature)
  • Documentation (improve or add documentation)

Key checklist

  • All tests pass: $ cargo test
  • The documentation builds and looks OK: $ cargo doc

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@alexdewar alexdewar requested review from Copilot and tsmbland June 17, 2025 14:48
Copy link
Copy Markdown
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 tidies up the process input code by refactoring functions to use a unified ProcessMap, while also transitioning input/output commodities to “other” commodities. Key changes include:

  • Removing redundant process ID arguments and simplifying API calls by using ProcessMap in process parameter, flow, and availability reading functions.
  • Updating tests and error messages to reflect the new commodity type “Other” and revised function signatures.
  • Adjusting the commodity type definitions in commodity.rs and updating the corresponding CSV example.

Reviewed Changes

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

Show a summary per file
File Description
src/input/process/parameter.rs Refactored to remove process_ids and use ProcessMap
src/input/process/flow.rs Refactored to remove process_ids and adjust error messages
src/input/process/availability.rs Updated parameter list to use ProcessMap and improved error context
src/input/process.rs Consolidated process map handling; updated commodity validations
src/commodity.rs Converted input/output commodity definitions into a single “Other” type
examples/simple/commodities.csv Updated commodity type identifier from ouc to oth

Copy link
Copy Markdown
Collaborator

@tsmbland tsmbland left a comment

Choose a reason for hiding this comment

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

LGTM!

@alexdewar alexdewar enabled auto-merge June 17, 2025 15:36
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 17, 2025

Codecov Report

Attention: Patch coverage is 85.71429% with 12 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@ff47a80). Learn more about missing BASE report.
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
src/input/process.rs 89.06% 1 Missing and 6 partials ⚠️
src/input/process/availability.rs 75.00% 0 Missing and 2 partials ⚠️
src/input/process/parameter.rs 66.66% 0 Missing and 2 partials ⚠️
src/input/process/flow.rs 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #629   +/-   ##
=======================================
  Coverage        ?   87.04%           
=======================================
  Files           ?       38           
  Lines           ?     3372           
  Branches        ?     3372           
=======================================
  Hits            ?     2935           
  Misses          ?      250           
  Partials        ?      187           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alexdewar alexdewar merged commit 4473c5d into main Jun 17, 2025
7 checks passed
@alexdewar alexdewar deleted the other-commodities branch June 17, 2025 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants