Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Use the default time pattern for persisting time series keys instead of the toString method #766

Open
t-ober opened this issue Mar 1, 2023 · 4 comments · May be fixed by #767
Open

Use the default time pattern for persisting time series keys instead of the toString method #766

t-ober opened this issue Mar 1, 2023 · 4 comments · May be fixed by #767
Assignees
@jo-bao
Labels
enhancement New feature or request

Comments

@t-ober
Copy link
Contributor

t-ober commented Mar 1, 2023

Currently the to string persistence outputs time strings with the [UTC] postfix which we can't easily parse. Use the default time pattern instead.
@t-ober t-ober added the enhancement New feature or request label Mar 1, 2023
@t-ober
Copy link
Contributor Author

t-ober commented Mar 1, 2023

Also I decided to simplify the time pattern to conform to the default time pattern we set up in the TimeUtils of the PowerSystemUtils

t-ober added a commit that referenced this issue Mar 1, 2023
@t-ober
Merge branch 'dev' into to/#766-use-default-time-pattern
84bde53
@t-ober
Copy link
Contributor Author

t-ober commented Mar 1, 2023 •
edited

@sebastian-peter : This is the discussion we had some time ago regarding ZonedTimeStamps I started a branch. But changing the toString method also changes the result output format (removing the [UTC] part). I'm willing to bite the bullet and change all the tests but first I wanted to get your opinion.

The logic changes are quite minor, have a look here: #767

@t-ober t-ober linked a pull request Mar 1, 2023 that will close this issue
Draft
@t-ober t-ober mentioned this issue Mar 14, 2023
Closed
@t-ober
Copy link
Contributor Author

t-ober commented Mar 14, 2023 •
edited

As per our discussion:

  1. Long term we would prefer LocalDateTime to avoid confusion and since we don't expect different time zones.
  2. Short term we might use UTC therefore adding the Z to the time pattern.
  3. Check time zone of our weather data.

For the short term solution we need to make sure that we actually respect the time zones in weather retrieval accordingly!!

@vickybung1 vickybung1 mentioned this issue Apr 24, 2023
Closed
@danielfeismann
Copy link
Member

Additional it might be also necessary to check PvModels needs of time format - e.g. in calcHourAnlgeOmega.

t-ober added a commit that referenced this issue Nov 28, 2023
@t-ober
Merge remote-tracking branch 'origin/dev' into to/#766-use-default-ti…
00acc4c 
…me-pattern

# Conflicts:
#	src/main/java/edu/ie3/datamodel/io/processor/Processor.java
#	src/main/java/edu/ie3/datamodel/io/source/csv/CsvTimeSeriesSource.java
#	src/test/groovy/edu/ie3/datamodel/io/source/csv/CsvTimeSeriesSourceTest.groovy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jo-bao jo-bao

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
3 participants
@t-ober @danielfeismann @jo-bao