Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Close multiple simple issues in PSY #1192

Merged
merged 11 commits into from
Sep 16, 2024
Merged

Close multiple simple issues in PSY #1192

merged 11 commits into from
Sep 16, 2024

Conversation

rodrigomha
Copy link
Contributor

@rodrigomha rodrigomha commented Sep 13, 2024

Closes #1175 via 692dfe0
Closes #1165 via 9dd3ef2
Closes #1162 via a62741c
Closes #1147 via 676e777
Closes #1160 via d954dee

@rodrigomha rodrigomha self-assigned this Sep 13, 2024
@@ -13,7 +13,6 @@ The `PowerSystems.jl` package provides a rigorous data model using Julia structu
## Version Advisory

- PowerSystems will work with Julia v1.6+.
- If you are planning to use `PowerSystems.jl` in your package, check the [roadmap to version 4.0](https://github.com/NREL-Sienna/PowerSystems.jl/projects/4) for upcoming changes
Copy link
Member

Choose a reason for hiding this comment

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

I need to make the version 5.0

Copy link

codecov bot commented Sep 13, 2024

Codecov Report

Attention: Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.63%. Comparing base (d954dee) to head (7fc254a).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
src/models/generated/MonitoredLine.jl 66.66% 2 Missing ⚠️
src/utils/print.jl 0.00% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1192   +/-   ##
=======================================
  Coverage   84.63%   84.63%           
=======================================
  Files         180      180           
  Lines        8264     8264           
=======================================
  Hits         6994     6994           
  Misses       1270     1270           
Flag Coverage Δ
unittests 84.63% <77.77%> (ø)

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

Files with missing lines Coverage Δ
src/base.jl 89.34% <ø> (ø)
src/models/generated/Line.jl 100.00% <100.00%> (ø)
src/parsers/power_models_data.jl 91.98% <100.00%> (ø)
src/utils/IO/system_checks.jl 83.82% <100.00%> (ø)
src/models/generated/MonitoredLine.jl 94.28% <66.66%> (ø)
src/utils/print.jl 82.14% <0.00%> (ø)

src/parsers/pm_io/data.jl Show resolved Hide resolved
@@ -346,9 +345,15 @@ function read_loadzones!(
load_zone_map[zone]["pd"] += get(load, "py", 0.0)
load_zone_map[zone]["qd"] += get(load, "qy", 0.0)
end

default_loadzone_naming = x -> string(x)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
default_loadzone_naming = x -> string(x)
default_loadzone_naming = string

Copy link
Member

Choose a reason for hiding this comment

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

good catch


default_loadzone_naming = x -> string(x)
# The formatter for loadzone_name should be a function that transform the LoadZone Int to a String
_get_name = get(kwargs, :loadzone_name_formatter, default_loadzone_naming)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we get unit tests for this?

@GabrielKS GabrielKS self-requested a review September 16, 2024 21:06
"psse_Benchmark_4ger_33_2015_sys";
loadzone_name_formatter = x -> string(3 * x),
)
lz_original = first(get_components(LoadZone, sys2))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you can safely assume that the result of get_components will be in the same order every time. Maybe get all the LoadZone names in each, sort those lists, and compare the lists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is only one load zone for that system, that's why I can use first

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh okay

"psse_Benchmark_4ger_33_2015_sys";
loadzone_name_formatter = x -> string(3 * x),
)
lz_original = first(get_components(LoadZone, sys2))
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick:

Suggested change
lz_original = first(get_components(LoadZone, sys2))
lz_original = only(get_components(LoadZone, sys2))

loadzone_name_formatter = x -> string(3 * x),
)
lz_original = first(get_components(LoadZone, sys2))
lz_new = first(get_components(LoadZone, sys3))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
lz_new = first(get_components(LoadZone, sys3))
lz_new = only(get_components(LoadZone, sys3))

Copy link
Collaborator

@GabrielKS GabrielKS left a comment

Choose a reason for hiding this comment

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

Looks good!

@jd-lara jd-lara merged commit 653a425 into main Sep 16, 2024
7 of 8 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
3 participants