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
38 stars 20 forks source link

MethodError: no method matching sort(::InfrastructureSystems.FlattenIteratorWrapper{PowerLoad}; by=var"#135#136"()) #281

Closed kdheepak closed 2 years ago

kdheepak commented 2 years ago

I'm unable to run the following:

        sort(
            get_components(PowerLoad, sys),
            by = load -> mean(values(get_time_series_array(Deterministic, load, "max_active_power")))
        )

The workaround is to wrap get_components(PowerLoad, sys) with collect(...).

daniel-thom commented 2 years ago

I don't think this is an issue. The only reason we have FlattenIteratorWrapper to extend Base.Iterators.Flatten with the length method for cases where we do actually know the length of the underlying iterables. You also can't call length(Base.Iterators.Flatten). I think that calling collect on the iterator is what you should have to do in order to get a single, sortable container. One of the goals of get_components is to provide a read-only iterator over existing containers rather than allocating new containers. If you have a different expectation, please elaborate.

kdheepak commented 2 years ago

I think your explanation makes sense about the current implementation and I’m happy to close this.

My opinion is that users reading the function name get_components or reading the docstring for it might have an expectation that this function returns a collection, and not a container that can yield a collection. Maybe a clarification in the docs is the only action item here.