search

NREL-Sienna / InfrastructureSystems.jl

Utility package for Sienna's simulation infrastructure
https://nrel-sienna.github.io/InfrastructureSystems.jl/
BSD 3-Clause "New" or "Revised" License
35 stars 20 forks source link

Resolve nameof vs strip_module_name #340

Closed daniel-thom closed 3 months ago

daniel-thom commented 3 months ago

This is a follow-on to #339. There were cases where we were using nameof (which removes parameteric types) where we should have used strip_module_name (which preserves parametric types). The best example is when printing a table of component counts. We were showing VariableReserve twice instead of showing both VariableReserve{ReserveDown} and VariableReserve{ReserveUp}.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 79.83%. Comparing base (b311f7f) to head (156469f). Report is 3 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/NREL-Sienna/InfrastructureSystems.jl/pull/340/graphs/tree.svg?width=650&height=150&src=pr&token=I73yjxYxgn&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna)](https://app.codecov.io/gh/NREL-Sienna/InfrastructureSystems.jl/pull/340?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna) ```diff @@ Coverage Diff @@ ## main #340 +/- ## ========================================== + Coverage 79.81% 79.83% +0.02% ========================================== Files 54 54 Lines 4335 4330 -5 ========================================== - Hits 3460 3457 -3 + Misses 875 873 -2 ``` | [Flag](https://app.codecov.io/gh/NREL-Sienna/InfrastructureSystems.jl/pull/340/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/NREL-Sienna/InfrastructureSystems.jl/pull/340/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna) | `79.83% <50.00%> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/NREL-Sienna/InfrastructureSystems.jl/pull/340?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna) | Coverage Δ | | |---|---|---| | [src/system\_data.jl](https://app.codecov.io/gh/NREL-Sienna/InfrastructureSystems.jl/pull/340?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna#diff-c3JjL3N5c3RlbV9kYXRhLmps) | `90.97% <100.00%> (ø)` | | | [src/utils/print.jl](https://app.codecov.io/gh/NREL-Sienna/InfrastructureSystems.jl/pull/340?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna#diff-c3JjL3V0aWxzL3ByaW50Lmps) | `52.38% <66.66%> (ø)` | | | [src/utils/utils.jl](https://app.codecov.io/gh/NREL-Sienna/InfrastructureSystems.jl/pull/340?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna#diff-c3JjL3V0aWxzL3V0aWxzLmps) | `62.64% <0.00%> (+0.04%)` | :arrow_up: | | [src/validation.jl](https://app.codecov.io/gh/NREL-Sienna/InfrastructureSystems.jl/pull/340?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=NREL-Sienna#diff-c3JjL3ZhbGlkYXRpb24uamw=) | `1.90% <0.00%> (ø)` | |
GabrielKS commented 3 months ago

These were the "more intricacies to figure out" I was talking about :)

GabrielKS commented 3 months ago

I had assumed that surely there was a way to get the parameters of a type in general, but it seems like maybe the only ways to do this without string manipulation use non-public interface.

daniel-thom commented 3 months ago

I had assumed that surely there was a way to get the parameters of a type in general, but it seems like maybe the only ways to do this without string manipulation use non-public interface.

Like this? https://github.com/NREL-Sienna/InfrastructureSystems.jl/blob/main/src/serialization.jl#L115

GabrielKS commented 3 months ago

I had assumed that surely there was a way to get the parameters of a type in general, but it seems like maybe the only ways to do this without string manipulation use non-public interface.

Like this? https://github.com/NREL-Sienna/InfrastructureSystems.jl/blob/main/src/serialization.jl#L115

I was under the impression that .parameters was not considered public interface.