Skip to content

Conversation

ckittl
Copy link
Member

@ckittl ckittl commented Feb 9, 2021

Resolves #274

@ckittl ckittl added the enhancement New feature or request label Feb 9, 2021
@ckittl ckittl self-assigned this Feb 9, 2021
@codecov
Copy link

codecov bot commented Feb 23, 2021

Codecov Report

Merging #276 (f7f8262) into dev (59607ab) will decrease coverage by 0.31%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev     #276      +/-   ##
============================================
- Coverage     78.29%   77.98%   -0.32%     
  Complexity     1950     1950              
============================================
  Files           247      249       +2     
  Lines          7911     7935      +24     
  Branches        747      749       +2     
============================================
- Hits           6194     6188       -6     
- Misses         1299     1321      +22     
- Partials        418      426       +8     
Impacted Files Coverage Δ Complexity Δ
...tamodel/models/timeseries/TimeSeriesContainer.java 0.00% <0.00%> (-56.42%) 0.00% <0.00%> (-9.00%)
...du/ie3/datamodel/models/timeseries/TimeSeries.java 25.00% <0.00%> (-5.00%) 3.00% <0.00%> (-1.00%)
...ls/timeseries/individual/IndividualTimeSeries.java 72.72% <0.00%> (-4.55%) 12.00% <0.00%> (-1.00%)
...3/datamodel/io/source/csv/CsvTimeSeriesSource.java 96.07% <0.00%> (-3.93%) 14.00% <0.00%> (+5.00%) ⬇️
.../ie3/datamodel/io/source/csv/CsvWeatherSource.java 80.26% <0.00%> (-1.45%) 20.00% <0.00%> (-2.00%)
.../ie3/datamodel/io/connectors/CsvFileConnector.java 77.38% <0.00%> (-0.79%) 33.00% <0.00%> (+5.00%) ⬇️
...l/models/timeseries/mapping/TimeSeriesMapping.java
...n/java/edu/ie3/datamodel/utils/TimeSeriesUtil.java 75.00% <0.00%> (ø) 2.00% <0.00%> (?%)
...odel/io/source/csv/CsvTimeSeriesMappingSource.java 100.00% <0.00%> (ø) 4.00% <0.00%> (?%)
...3/datamodel/io/source/TimeSeriesMappingSource.java 46.66% <0.00%> (ø) 1.00% <0.00%> (?%)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a2c0f5...f7f8262. Read the comment docs.

@ckittl
Copy link
Member Author

ckittl commented Feb 23, 2021

!test

@ckittl ckittl requested a review from a team February 23, 2021 13:35
@ckittl ckittl marked this pull request as ready for review February 23, 2021 13:35
@ckittl
Copy link
Member Author

ckittl commented Mar 1, 2021

What did I do here?

I modularized the time series sources a bit further:

  • Having a distinct mapping source, that provides information on which time series is there for which model as well as the types of time series aka. meta information
  • The time series source itself only provides information about one time series at a time (including the possibility to query results for periods in time or specific instances in time)
  • The CsvTimeSeriesSource` has a factory to build correct source from given meta information

@ckittl ckittl marked this pull request as draft March 1, 2021 09:49
@ckittl ckittl marked this pull request as ready for review March 1, 2021 10:16
Copy link
Member

@johanneshiry johanneshiry left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution. 👍🏼
Coverage is also fine for now, as mainly equals or exceptions are not tested.
Just had a few minor remarks that should be fast addressable. :-)

@ckittl ckittl requested a review from johanneshiry March 4, 2021 07:16
@johanneshiry
Copy link
Member

!test

@johanneshiry johanneshiry merged commit 854d463 into dev Mar 4, 2021
@johanneshiry johanneshiry deleted the ck/#274-reworkTimeSeriesSource branch March 4, 2021 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restructure time series sources

2 participants