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.

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

Cse valid/glsk merged #92

Merged
merged 9 commits into from
Mar 29, 2023
Merged

Cse valid/glsk merged #92

merged 9 commits into from
Mar 29, 2023

Conversation

vbochetRTE
Copy link
Collaborator

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem ? If so, link to this issue using '#XXX' and skip the rest

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

What is the current behavior? (You can also link to an open issue here)

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change or deprecate an API? If yes, check the following:

  • The Breaking Change or Deprecated label has been added
  • The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

Other information:

(if any of the questions/checkboxes don't apply, please delete them entirely)

@vbochetRTE vbochetRTE force-pushed the cse-valid/glsk-merged branch from b1b2bc7 to 7cf275b Compare March 21, 2023 16:19
Signed-off-by: Vincent BOCHET <vincent.bochet@rte-france.com>
Signed-off-by: Vincent BOCHET <vincent.bochet@rte-france.com>
Signed-off-by: Vincent BOCHET <vincent.bochet@rte-france.com>
Signed-off-by: Vincent BOCHET <vincent.bochet@rte-france.com>
…outarea countries

Signed-off-by: Vincent BOCHET <vincent.bochet@rte-france.com>
Signed-off-by: Vincent BOCHET <vincent.bochet@rte-france.com>
@vbochetRTE vbochetRTE force-pushed the cse-valid/glsk-merged branch from 7cf275b to 2689b5a Compare March 21, 2023 16:43
@vbochetRTE vbochetRTE requested a review from phiedw March 22, 2023 11:48
Signed-off-by: Vincent BOCHET <vincent.bochet@rte-france.com>
Signed-off-by: Vincent BOCHET <vincent.bochet@rte-france.com>
private static Map<String, List<GlskPoint>> getGlskPointsFromTimeSeries(List<TimeSeriesType> timeSeries) {
Map<String, List<GlskPoint>> cseGlskPoints = new TreeMap<>();
if (timeSeries == null) {
return cseGlskPoints;
Copy link
Collaborator

Choose a reason for hiding this comment

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

should a warning be logged / an exception be thrown here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

An exception, I'm sure not.
A warning, not sure if it is useful or not... Theoretically the timeSeries variable here should never be null, as we check in CseGlskDocumentImporter.canImport() method that the document contains at least one element named . It could possibly be null for timeSeriesExport, if no markup exists in the document. But it will happen for every "standard" GLSK file handling (I mean here, no Export Corner handling), so I don't think we need to warn the user systematically.

}

private static Map<String, List<GlskPoint>> getGlskPointsFromTimeSeries(List<TimeSeriesType> timeSeries) {
Map<String, List<GlskPoint>> cseGlskPoints = new TreeMap<>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe you could rename it to cseGlskPointsPerArea


private static final List<Class<?>> STANDARD_BLOCK_CLASSES = List.of(ManualGSKBlockType.class, PropGSKBlockType.class, PropLSKBlockType.class, ReserveGSKBlockType.class);

public CseGlskPoint(TimeSeriesType timeSerie) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

timeSeries* (singular is series, which can make devs a bit confusing i guess)

Comment on lines +126 to +143
String italyEIC = new CountryEICode(Country.IT).getCode();
List<String> countriesInArea = new ArrayList<>();
List<String> countriesOutArea = new ArrayList<>();

calculationDirections.stream()
.map(cd -> cd.getInArea().getV())
.filter(countryEIC -> !countryEIC.equals(italyEIC))
.forEach(countriesInArea::add);

countriesInArea.add(italyEIC);

calculationDirections.stream()
.map(cd -> cd.getOutArea().getV())
.filter(countryEIC -> !countryEIC.equals(italyEIC))
.forEach(countriesOutArea::add);

return Map.of(COUNTRIES_IN_AREA_KEY, countriesInArea,
COUNTRIES_OUT_AREA_KEY, countriesOutArea);
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is Italy a special case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For Export Corner developments, the CalculationDirections (representing which countries have activated Export Corner excahnges or not) should contain only directions from a country to Italy or vice-versa. E.g.: {InArea: France, OutArea: Italy} if France has activated Export Corner exchanges, or {InArea: Italy, OutArea: France} if France has not activated Export Corner exchanges.
There is no sense to put {InArea: Italy, OutArea: Italy} because no exchange is made from Italy to itself.
But we need to handle Italy because we need to retrieve its TimeSeries. As the rules for Italy (concerning TimeSeries retrieval) are the same as the rules for countries having activated Export Corner exchanges, I decided to put it in the countriesInArea List.

Signed-off-by: Vincent BOCHET <vincent.bochet@rte-france.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

87.9% 87.9% Coverage
0.0% 0.0% Duplication

@phiedw phiedw merged commit e73f815 into main Mar 29, 2023
@phiedw phiedw deleted the cse-valid/glsk-merged branch March 29, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants