Jd/add infos #316

jd-lara · 2023-11-29T22:01:43Z

No description provided.

src/geographic_info.jl

src/infos.jl

src/InfrastructureSystems.jl

src/component.jl

src/system_data.jl

Co-authored-by: Daniel Thom <daniel.thom@nrel.gov>

src/supplemental_attributes.jl

Co-authored-by: Daniel Thom <daniel.thom@nrel.gov>

jd-lara · 2023-12-12T05:47:27Z

@daniel-thom I addressed most of the comments. There are a few failing tests related to the time series and the serialization

jd-lara · 2023-12-18T23:46:02Z

Tests are expected to fail on serialization/deserialization

github-actions · 2023-12-18T23:46:06Z

src/utils/test.jl

+end
+
+function TestSupplemental(;
+    value::Float64 = 0.0,


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

value::Float64 = 0.0,

value::Float64=0.0,

github-actions · 2023-12-18T23:46:06Z

src/utils/test.jl

+    value::Float64 = 0.0,
+    component_uuids::Set{UUIDs.UUID}=Set{UUIDs.UUID}(),
+)
+    return TestSupplemental(value, component_uuids, InfrastructureSystemsInternal(), TimeSeriesContainer())


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

return TestSupplemental(value, component_uuids, InfrastructureSystemsInternal(), TimeSeriesContainer())

return TestSupplemental(

value,

component_uuids,

InfrastructureSystemsInternal(),

TimeSeriesContainer(),

)

github-actions · 2023-12-18T23:46:07Z

test/test_supplemental_attributes.jl

+
+

[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

github-actions

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

JuliaFormatter

src/time_series_interface.jl|587|
src/time_series_interface.jl|602|
src/utils/print.jl|33|
src/utils/test.jl|110|
test/test_supplemental_attributes.jl|85|
test/test_system_data.jl|255|

github-actions · 2023-12-18T23:50:46Z

src/components.jl

    data::ComponentsByType
    time_series_storage::TimeSeriesStorage
    validation_descriptors::Vector
 end

+get_display_string(::Components) = "components"
+
 function Components(time_series_storage::TimeSeriesStorage, validation_descriptors=nothing)


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

function Components(time_series_storage::TimeSeriesStorage, validation_descriptors=nothing)

function Components(

time_series_storage::TimeSeriesStorage,

validation_descriptors = nothing,

)

github-actions · 2023-12-18T23:50:46Z

src/components.jl

@@ -388,6 +362,7 @@ function set_name!(
    set_name_internal!(component, name)
    components.data[T][name] = component
    @debug "Changed the name of component $(summary(component))" _group = LOG_GROUP_SYSTEM
+    return
 end

 function compare_values(x::Components, y::Components; compare_uuids=false)


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

function compare_values(x::Components, y::Components; compare_uuids=false)

function compare_values(x::Components, y::Components; compare_uuids = false)

github-actions · 2023-12-18T23:50:46Z

src/geographic_supplemental_attribute.jl

+    geo_json::Dict{String, Any}=Dict{String, Any}(),
+    component_uuids::Set{UUIDs.UUID}=Set{UUIDs.UUID}(),


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

geo_json::Dict{String, Any}=Dict{String, Any}(),

component_uuids::Set{UUIDs.UUID}=Set{UUIDs.UUID}(),

geo_json::Dict{String, Any} = Dict{String, Any}(),

component_uuids::Set{UUIDs.UUID} = Set{UUIDs.UUID}(),

github-actions · 2023-12-18T23:50:46Z

src/supplemental_attributes.jl

+function _add_supplemental_attribute!(
+    supplemental_attributes::SupplementalAttributes,
+    supplemental_attribute::T;
+    allow_existing_time_series=false,


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

allow_existing_time_series=false,

allow_existing_time_series = false,

github-actions · 2023-12-18T23:50:46Z

src/supplemental_attributes.jl

+function get_supplemental_attributes(
+    ::Type{T},
+    supplemental_attributes::SupplementalAttributes,
+    filter_func::Union{Nothing, Function}=nothing,


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

filter_func::Union{Nothing, Function}=nothing,

filter_func::Union{Nothing, Function} = nothing,

github-actions · 2023-12-18T23:50:48Z

src/time_series_interface.jl

+            len=len,
+            ignore_scaling_factors=ignore_scaling_factors,


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

len=len,

ignore_scaling_factors=ignore_scaling_factors,

len = len,

ignore_scaling_factors = ignore_scaling_factors,

github-actions · 2023-12-18T23:50:48Z

src/time_series_interface.jl

+    start_time::Union{Nothing, Dates.DateTime}=nothing;
+    len::Union{Nothing, Int}=nothing,
+    ignore_scaling_factors=false,


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

start_time::Union{Nothing, Dates.DateTime}=nothing;

len::Union{Nothing, Int}=nothing,

ignore_scaling_factors=false,

start_time::Union{Nothing, Dates.DateTime} = nothing;

len::Union{Nothing, Int} = nothing,

ignore_scaling_factors = false,

github-actions · 2023-12-18T23:50:48Z

src/time_series_interface.jl

+            len=len,
+            ignore_scaling_factors=ignore_scaling_factors,


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

len=len,

ignore_scaling_factors=ignore_scaling_factors,

len = len,

ignore_scaling_factors = ignore_scaling_factors,

github-actions · 2023-12-18T23:50:49Z

src/time_series_interface.jl

+end
+
+function _make_time_array(component, time_series, start_time, len, ignore_scaling_factors)
+    ta = make_time_array(time_series, start_time; len=len)


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

ta = make_time_array(time_series, start_time; len=len)

ta = make_time_array(time_series, start_time; len = len)

github-actions · 2023-12-18T23:50:49Z

src/time_series_interface.jl

+    name_mapping::Union{Nothing, Dict{Tuple{String, String}, String}}=nothing,
+    scaling_factor_multiplier_mapping::Union{Nothing, Dict{String, String}}=nothing,


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

name_mapping::Union{Nothing, Dict{Tuple{String, String}, String}}=nothing,

scaling_factor_multiplier_mapping::Union{Nothing, Dict{String, String}}=nothing,

name_mapping::Union{Nothing, Dict{Tuple{String, String}, String}} = nothing,

scaling_factor_multiplier_mapping::Union{Nothing, Dict{String, String}} = nothing,

daniel-thom · 2023-12-13T00:36:20Z

src/component.jl

+        attribute_container[T] = Set{T}()
+    end
+    push!(attribute_container[T], attribute)
+    @debug "SupplementalAttribute type $T with UUID $(get_uuid(attribute)) stored in component $(get_name(component))"


Do we need a LOG_GROUP for this?

These are already implemented.

daniel-thom · 2023-12-13T00:37:35Z

src/component.jl

+    container = get_supplemental_attributes_container(component)
+    for attribute_set in values(container)
+        for i in attribute_set
+            delete!(get_components_uuids(i), get_uuid(component))


One plural should be sufficient: get_component_uuids.

These are fixed already

daniel-thom · 2023-12-13T00:39:16Z

src/component.jl

+    if !haskey(container, T)
+        throw(
+            ArgumentError(
+                "supplemental attribute type $T is not stored in component $(get_name(component))",


Related to comments in the last round, I think you should use $(summary(component)) so that the component type shows up in the message.

These are fixed already

src/geographic_supplemental_attribute.jl

daniel-thom · 2023-12-21T18:33:50Z

src/component.jl

+    attribute::T,
+) where {T <: InfrastructureSystemsSupplementalAttribute}
+    attach_component!(attribute, component)
+    attribute_container = get_supplemental_attributes_container(component)


Can this call attach_component!? The code here and there is different. It seems not great to modify the attribute inside the component code.

src/system_data.jl

daniel-thom · 2023-12-21T18:48:49Z

src/system_data.jl

+    for info in attributes
+        for c_uuid in get_component_uuids(info)
+            comp = get_component(data, c_uuid)
+            delete!(get_supplemental_attributes_container(comp), info)


Same above, I think this could call a component method.

daniel-thom · 2023-12-21T18:49:59Z

src/time_series_interface.jl

@@ -0,0 +1,606 @@
+const SUPPORTED_TIME_SERIES_TYPES =


For type aliases don’t we normally use pascal case? SupportedTimeSeriesTypes?

daniel-thom · 2023-12-21T19:01:43Z

src/system_data.jl

+    return remove_supplemental_attribute!(data.attributes, info)
+end
+
+function remove_supplemental_attributes!(


I have concerns about this function because of the implicit disconnection from components. Since the user can only add attributes through a component, shouldn’t they only be able to remove attributes through a component?

This contradicts previous requests to have a method to do this

This method will clear all attributed of a particular type

github-actions · 2023-12-21T19:55:53Z

src/system_data.jl

[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

in_memory = time_series_in_memory,

directory = time_series_directory,

compression = compression,

github-actions · 2023-12-21T19:55:54Z

src/time_series_interface.jl

+    component::SUPPORTED_TIME_SERIES_TYPES,
+    ts_metadata::T,
+    ts::TimeSeriesData;
+    skip_if_present=false,


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

skip_if_present=false,

skip_if_present = false,

github-actions · 2023-12-21T19:55:54Z

src/time_series_interface.jl

+        get_name(ts_metadata),
+        ts,
+    )
+    add_time_series!(component, ts_metadata, skip_if_present=skip_if_present)


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

add_time_series!(component, ts_metadata, skip_if_present=skip_if_present)

add_time_series!(component, ts_metadata; skip_if_present = skip_if_present)

github-actions · 2023-12-21T19:55:54Z

src/utils/print.jl

    println(io, "==========")
    println(io, "Num components: $num_components")
    if num_components > 0
        println(io)
-        show_components_table(io, components, backend=Val(:auto))
+        show_components_table(io, container, backend=Val(:auto))


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

show_components_table(io, container, backend=Val(:auto))

show_components_table(io, container; backend = Val(:auto))

github-actions · 2023-12-21T19:55:54Z

src/utils/test.jl

+
+function TestSupplemental(;
+    value::Float64 = 0.0,
+    component_uuids::Set{UUIDs.UUID}=Set{UUIDs.UUID}(),


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

component_uuids::Set{UUIDs.UUID}=Set{UUIDs.UUID}(),

component_uuids::Set{UUIDs.UUID} = Set{UUIDs.UUID}(),

github-actions · 2023-12-21T19:55:54Z

test/test_supplemental_attributes.jl

+    ta = TimeSeries.TimeArray(range(initial_time; length=24, step=resolution), ones(24))
+    ts = IS.SingleTimeSeries(data=ta, name="test")


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

ta = TimeSeries.TimeArray(range(initial_time; length=24, step=resolution), ones(24))

ts = IS.SingleTimeSeries(data=ta, name="test")

ta = TimeSeries.TimeArray(range(initial_time; length = 24, step = resolution), ones(24))

ts = IS.SingleTimeSeries(; data = ta, name = "test")

github-actions · 2023-12-21T19:55:54Z

test/test_system_data.jl

+    ta = TimeSeries.TimeArray(range(initial_time; length=24, step=resolution), ones(24))
+    ts = IS.SingleTimeSeries(data=ta, name="test")


[JuliaFormatter] _{reported by reviewdog 🐶}

Suggested change

ta = TimeSeries.TimeArray(range(initial_time; length=24, step=resolution), ones(24))

ts = IS.SingleTimeSeries(data=ta, name="test")

ta = TimeSeries.TimeArray(range(initial_time; length = 24, step = resolution), ones(24))

ts = IS.SingleTimeSeries(; data = ta, name = "test")

codecov · 2023-12-22T00:01:51Z

Codecov Report

Attention: 99 lines in your changes are missing coverage. Please review.

Comparison is base (43d40cc) 78.08% compared to head (deebdbc) 78.35%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #316      +/-   ##
==========================================
+ Coverage   78.08%   78.35%   +0.27%     
==========================================
  Files          44       49       +5     
  Lines        3737     3964     +227     
==========================================
+ Hits         2918     3106     +188     
- Misses        819      858      +39

Flag	Coverage Δ
unittests	`78.35% <83.13%> (+0.27%)`	⬆️

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

Files	Coverage Δ
src/components.jl	`90.78% <100.00%> (-1.68%)`	⬇️
src/containers.jl	`100.00% <100.00%> (ø)`
src/deterministic.jl	`82.08% <100.00%> (-0.27%)`	⬇️
src/deterministic_single_time_series.jl	`76.27% <ø> (ø)`
src/hdf5_time_series_storage.jl	`91.10% <100.00%> (ø)`
src/in_memory_time_series_storage.jl	`73.22% <100.00%> (ø)`
src/internal.jl	`80.70% <100.00%> (+1.75%)`	⬆️
src/scenarios.jl	`84.31% <100.00%> (ø)`
src/time_series_cache.jl	`92.56% <100.00%> (ø)`
src/time_series_container.jl	`76.78% <ø> (ø)`
... and 24 more

jd-lara added 17 commits November 21, 2023 16:30

add info types

16f87d2

generalize containers

bb09978

add geographic info as example

390e9b1

fixes to compat

f91f19e

integrate info to components

24d2cc2

WIP integrating infos closer in the system data

592a804

add some additional functionality for geo info

fda144e

add info interface

e5812b4

improve info interface

b532839

WIP add testing

caa18ef

fix push! call

d48e685

fix a very old typo

db109e8

update printing methods

d14f4bf

add more info capabilities

47d1c70

add component and info removal

626cf93

update the methods for geo infos

d400498

improve testing and docs

41fc858

jd-lara requested a review from daniel-thom November 29, 2023 22:01

jd-lara mentioned this pull request Nov 29, 2023

Update Project.toml #315

Closed

fix typo

97b5f63

claytonpbarrows reviewed Nov 30, 2023

View reviewed changes

src/geographic_info.jl Outdated Show resolved Hide resolved

jd-lara added 3 commits November 30, 2023 11:26

fix details

ec52c29

implement system interface

597160e

use types directly

91f4e95

github-actions bot reviewed Nov 30, 2023

View reviewed changes

src/infos.jl Outdated Show resolved Hide resolved

rename info -> supplemental_attribute

b4a3d4b

daniel-thom reviewed Dec 5, 2023

View reviewed changes

jd-lara and others added 3 commits December 11, 2023 21:26

Update src/InfrastructureSystems.jl

d8c1439

Co-authored-by: Daniel Thom <daniel.thom@nrel.gov>

Update src/component.jl

904066f

Co-authored-by: Daniel Thom <daniel.thom@nrel.gov>

name replace

a1e881b

github-actions bot reviewed Dec 12, 2023

View reviewed changes

src/supplemental_attributes.jl Outdated Show resolved Hide resolved

jd-lara and others added 3 commits December 11, 2023 23:05

formatter

f2855b1

Update src/system_data.jl

fc75db6

Co-authored-by: Daniel Thom <daniel.thom@nrel.gov>

fix almost all tests

be0e6c5

jd-lara self-assigned this Dec 12, 2023

jd-lara added the enhancement New feature or request label Dec 12, 2023

jd-lara added 2 commits December 18, 2023 16:09

address PR comments

01c7911

add time series testing

7a8de74

jd-lara requested a review from daniel-thom December 18, 2023 23:45

jd-lara marked this pull request as ready for review December 18, 2023 23:45

github-actions bot reviewed Dec 18, 2023

View reviewed changes

update formatter action

4e2d04a

github-actions bot reviewed Dec 18, 2023

View reviewed changes

daniel-thom reviewed Dec 21, 2023

View reviewed changes

jd-lara added 2 commits December 21, 2023 13:53

address PR comments

cd73f60

address comments

7c31557

github-actions bot reviewed Dec 21, 2023

View reviewed changes

jd-lara added 6 commits December 21, 2023 14:06

address comments

0f6b54d

use detach methods

155ea39

address PR comments

3a0d1b7

more pr comments

423874f

fix serialization

1192753

formatter

deebdbc

daniel-thom approved these changes Dec 22, 2023

View reviewed changes

jd-lara merged commit 2f8a2dc into main Dec 22, 2023
10 checks passed

jd-lara deleted the jd/add_infos branch December 22, 2023 01:03

	function compare_values(x::Components, y::Components; compare_uuids=false)
	function compare_values(x::Components, y::Components; compare_uuids = false)

		geo_json::Dict{String, Any}=Dict{String, Any}(),
		component_uuids::Set{UUIDs.UUID}=Set{UUIDs.UUID}(),

	allow_existing_time_series=false,
	allow_existing_time_series = false,

	filter_func::Union{Nothing, Function}=nothing,
	filter_func::Union{Nothing, Function} = nothing,

	ta = make_time_array(time_series, start_time; len=len)
	ta = make_time_array(time_series, start_time; len = len)

		name_mapping::Union{Nothing, Dict{Tuple{String, String}, String}}=nothing,
		scaling_factor_multiplier_mapping::Union{Nothing, Dict{String, String}}=nothing,

+        in_memory = time_series_in_memory,
+        directory = time_series_directory,
+        compression = compression,

	add_time_series!(component, ts_metadata, skip_if_present=skip_if_present)
	add_time_series!(component, ts_metadata; skip_if_present = skip_if_present)

	show_components_table(io, container, backend=Val(:auto))
	show_components_table(io, container; backend = Val(:auto))

	component_uuids::Set{UUIDs.UUID}=Set{UUIDs.UUID}(),
	component_uuids::Set{UUIDs.UUID} = Set{UUIDs.UUID}(),

		ta = TimeSeries.TimeArray(range(initial_time; length=24, step=resolution), ones(24))
		ts = IS.SingleTimeSeries(data=ta, name="test")

Jd/add infos #316

Jd/add infos #316

Conversation

jd-lara commented Nov 29, 2023

jd-lara commented Dec 12, 2023

jd-lara commented Dec 18, 2023

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

github-actions bot Dec 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot Dec 21, 2023

Choose a reason for hiding this comment

github-actions bot Dec 21, 2023

Choose a reason for hiding this comment

github-actions bot Dec 21, 2023

Choose a reason for hiding this comment

github-actions bot Dec 21, 2023

Choose a reason for hiding this comment

github-actions bot Dec 21, 2023

Choose a reason for hiding this comment

github-actions bot Dec 21, 2023

Choose a reason for hiding this comment

github-actions bot Dec 21, 2023

Choose a reason for hiding this comment

codecov bot commented Dec 22, 2023 • edited Loading

Codecov Report

codecov bot commented Dec 22, 2023 •

edited

Loading