-
Notifications
You must be signed in to change notification settings - Fork 39
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
Support writing IIDM-XML network extensions in previous versions #1138
Conversation
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
7b9569c
to
3bcf990
Compare
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
iidm/iidm-converter-api/src/main/java/com/powsybl/iidm/export/ExportOptions.java
Outdated
Show resolved
Hide resolved
Could please add:
|
…on exists before writing Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like a second reviewer on this PR
commons/src/main/java/com/powsybl/commons/extensions/ExtensionXmlSerializer.java
Outdated
Show resolved
Hide resolved
iidm/iidm-xml-converter/src/main/java/com/powsybl/iidm/xml/NetworkXml.java
Show resolved
Hide resolved
...n/java/com/powsybl/iidm/xml/extensions/AbstractVersionableNetworkExtensionXmlSerializer.java
Show resolved
Hide resolved
iidm/iidm-xml-converter/src/main/java/com/powsybl/iidm/xml/XMLExporter.java
Show resolved
Hide resolved
...xml-converter/src/test/java/com/powsybl/iidm/xml/IdentifiableExtensionXmlSerializerTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MioRtia thanks for your work on this PR! Overall it looks good to me. I've just left comments when I didn't understand what you did.
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
public void testThrowErrorUncompatibleExtensionVersion() { | ||
public void testNotLatestVersionTerminalExtension() throws IOException { | ||
// import XIIDM file with loadMock v1.2 | ||
Network network = NetworkXml.read(getClass().getResourceAsStream("/V1_1/eurostag-tutorial-example1-with-loadMockExt-1_2.xml")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you use getVersionDir(IidmXmlVersion.V1_1)
also there?
Maybe we should introduce a method somewhere to get a network resource based on a version and a filename:
InputStream getNetworkAsStream(String filename, IidmVersion version) {
return getClass().getResourceAsStream(version + "/" + filename);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will make the change you suggested and introduce the new method in a new PR to prevent confusion in this one.
iidm/iidm-converter-api/src/main/java/com/powsybl/iidm/export/ExportOptions.java
Show resolved
Hide resolved
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Kudos, SonarCloud Quality Gate passed!
|
Signed-off-by: RALAMBOTIANA MIORA miora.ralambotiana@rte-france.com
Please check if the PR fulfills these requirements (please use
'[x]'
to check the checkboxes, or submit the PR and then click the checkboxes)What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the current behavior? (You can also link to an open issue here)
Network extensions can only be serialized in their latest version.
What is the new behavior (if this is a feature change)?
A user can choose the version in which they want to serialize their network extensions.
Does this PR introduce a breaking change or deprecate an API? If yes, check the following:
No