Skip to content

Add dicttype arg to JSON.parsefile calls throughout code#604

Open
hdunham wants to merge 4 commits into
developfrom
dict-type
Open

Add dicttype arg to JSON.parsefile calls throughout code#604
hdunham wants to merge 4 commits into
developfrom
dict-type

Conversation

@hdunham

@hdunham hdunham commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Please check if the PR fulfills these requirements

  • CHANGELOG.md is updated
  • Docs have been added / updated if applicable
  • Any new packages have been added to the [compat] section of Project.toml
  • REopt version number is updated in Project.toml and CHANGELOG.md if merging into master (do this right before merging)
  • Tests for the changes have been added. These tests should compare against expected values that are calculated independently of running REopt. Tests should also include garbage collection (see other tests for examples).
  • Any new REopt outputs and required inputs have been added to the corresponding Django model in the REopt API

Replace this with a description of the changes (can just copy from CHANGELOG.md)

hdunham added 3 commits May 1, 2026 12:34
solves arg type error when passed to next function as json object but expecting dict
@hdunham hdunham requested a review from Copilot May 20, 2026 23:15
@hdunham hdunham changed the title Dict type Add dicttype arg to JSON.parsefile calls throughout code May 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Ensures JSON scenario parsing consistently produces Dict{String,Any} (instead of JSON.Object) when loading scenario files, avoiding MethodError when passing parsed JSON into constructors like Scenario(d::Dict) / MPCScenario(d::Dict).

Changes:

  • Added dicttype = Dict{String, Any} to JSON.parsefile calls across REopt and MPC file-path entry points.
  • Updated PV defaults JSON loading to use the same dicttype.
  • Added a CHANGELOG entry describing the fix.

Reviewed changes

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

Show a summary per file
File Description
src/mpc/model.jl Forces parsed MPC scenario JSON into a Dict{String,Any} before constructing MPCScenario.
src/mpc/inputs.jl Forces parsed MPC inputs JSON into a Dict{String,Any} before constructing MPCScenario.
src/core/scenario.jl Ensures Scenario(fp::String) parses JSON into a Dict{String,Any}.
src/core/reopt.jl Ensures run_reopt(..., fp) and build_reopt!(..., fp) parse JSON into a Dict{String,Any}.
src/core/reopt_inputs.jl Ensures REoptInputs(fp::String) parses JSON into a Dict{String,Any} (incl. docstring example).
src/core/pv.jl Ensures PV defaults JSON is parsed into a Dict{String,Any}.
CHANGELOG.md Documents the fix in the changelog.

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

Comment thread CHANGELOG.md
Comment thread src/core/reopt.jl
@zolanaj zolanaj self-requested a review June 23, 2026 16:42

@zolanaj zolanaj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @hdunham! This looks good as is, and when testing this branch on the API all tests are passing.

This is good to merge but one question: do we want to update the test suite so that the JSON.parsefile(...) calls within are updated to call this new function?

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