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

Add new overload for loadNetwork and improve code in Importers #1986

Merged
merged 4 commits into from
Mar 1, 2022
Merged

Add new overload for loadNetwork and improve code in Importers #1986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
42cfa52
Add new overload for loadNetwork and improve code in Importers
miovd Feb 22, 2022
39b6364
Fix code smells
miovd Feb 25, 2022
70c9529
Add test
miovd Feb 25, 2022
4dcb568
Merge branch 'main' into add_improve_importers
miovd Mar 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 35 additions & 26 deletions iidm/iidm-converter-api/src/main/java/com/powsybl/iidm/import_/Importers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public final class Importers {

private static final Supplier<ImportConfig> CONFIG = Suppliers.memoize(ImportConfig::load);

private static final String UNSUPPORTED_FILE_FORMAT_OR_INVALID_FILE = "Unsupported file format or invalid file.";

private Importers() {
}

Expand Down Expand Up @@ -331,7 +333,7 @@ private static void doImport(ReadOnlyDataSource dataSource, Importer importer, P
if (listener != null) {
listener.accept(dataSource);
}
Network network = reporter == Reporter.NO_OP ? importer.importData(dataSource, NetworkFactory.findDefault(), parameters) : importer.importData(dataSource, NetworkFactory.findDefault(), parameters, reporter);
Network network = importer.importData(dataSource, NetworkFactory.findDefault(), parameters, reporter);
consumer.accept(network);
} catch (Exception e) {
LOGGER.error(e.toString(), e);
Expand Down Expand Up @@ -375,19 +377,12 @@ public static void importAll(Path dir, Importer importer, boolean parallel, Prop
if (parallel) {
ExecutorService executor = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
try {
List<Future<?>> futures;
if (reporter == Reporter.NO_OP) {
futures = dataSources.stream()
.map(ds -> executor.submit(() -> doImport(ds, importer, parameters, consumer, listener, reporter)))
.collect(Collectors.toList());
} else {
futures = dataSources.stream()
List<Future<?>> futures = dataSources.stream()
.map(ds -> {
Reporter child = createSubReporter(reporter, ds);
return executor.submit(() -> doImport(ds, importer, parameters, consumer, listener, child));
})
.collect(Collectors.toList());
}
for (Future<?> future : futures) {
future.get();
}
Expand All @@ -396,11 +391,7 @@ public static void importAll(Path dir, Importer importer, boolean parallel, Prop
}
} else {
for (ReadOnlyDataSource dataSource : dataSources) {
if (reporter == Reporter.NO_OP) {
doImport(dataSource, importer, parameters, consumer, listener, reporter);
} else {
doImport(dataSource, importer, parameters, consumer, listener, createSubReporter(reporter, dataSource));
}
doImport(dataSource, importer, parameters, consumer, listener, createSubReporter(reporter, dataSource));
}
}
}
Expand Down Expand Up @@ -459,6 +450,10 @@ public static Importer findImporter(ReadOnlyDataSource dataSource, ComputationMa
return findImporter(dataSource, LOADER.get(), computationManager, CONFIG.get());
}

public static Importer findImporter(ReadOnlyDataSource dataSource) {
return findImporter(dataSource, LocalComputationManager.getDefault());
}

/**
* Loads a network from the specified file, trying to guess its format.
*
Expand All @@ -474,13 +469,9 @@ public static Network loadNetwork(Path file, ComputationManager computationManag
ReadOnlyDataSource dataSource = createDataSource(file);
Importer importer = findImporter(dataSource, loader, computationManager, config);
if (importer != null) {
if (reporter == Reporter.NO_OP) {
return importer.importData(dataSource, NetworkFactory.findDefault(), parameters);
} else {
return importer.importData(dataSource, NetworkFactory.findDefault(), parameters, reporter);
}
return importer.importData(dataSource, NetworkFactory.findDefault(), parameters, reporter);
}
throw new PowsyblException("Unsupported file format or invalid file.");
throw new PowsyblException(UNSUPPORTED_FILE_FORMAT_OR_INVALID_FILE);
}

/**
Expand Down Expand Up @@ -538,6 +529,7 @@ public static Network loadNetwork(String file) {

/**
* Loads a network from a raw input stream, trying to guess the format from the specified filename.
* Please note that the input stream must be from a simple file, not a zipped one.
*
* @param filename The name of the file to be imported.
* @param data The raw data from which the network should be loaded
Expand All @@ -553,17 +545,14 @@ public static Network loadNetwork(String filename, InputStream data, Computation
dataSource.putData(filename, data);
Importer importer = findImporter(dataSource, loader, computationManager, config);
if (importer != null) {
if (reporter == Reporter.NO_OP) {
return importer.importData(dataSource, NetworkFactory.findDefault(), parameters);
} else {
return importer.importData(dataSource, NetworkFactory.findDefault(), parameters, reporter);
}
return importer.importData(dataSource, NetworkFactory.findDefault(), parameters, reporter);
}
throw new PowsyblException("Unsupported file format or invalid file.");
throw new PowsyblException(UNSUPPORTED_FILE_FORMAT_OR_INVALID_FILE);
}

/**
* Loads a network from a raw input stream, trying to guess the format from the specified filename.
* Please note that the input stream must be from a simple file, not a zipped one.
*
* @param filename The name of the file to be imported.
* @param data The raw data from which the network should be loaded
Expand All @@ -580,6 +569,7 @@ public static Network loadNetwork(String filename, InputStream data, Computation
/**
* Loads a network from a raw input stream, trying to guess the format from the specified filename,
* and using importers and post processors defined as services.
* Please note that the input stream must be from a simple file, not a zipped one.
*
* @param filename The name of the file to be imported.
* @param data The raw data from which the network should be loaded
Expand All @@ -597,6 +587,7 @@ public static Network loadNetwork(String filename, InputStream data, Computation
* and using importers and post processors defined as services.
* Import will be performed using import configuration defined in default platform config,
* and with no importer-specific parameters.
* Please note that the input stream must be from a simple file, not a zipped one.
*
* @param filename The name of the file to be imported.
* @param data The raw data from which the network should be loaded
Expand All @@ -614,6 +605,7 @@ public static Network loadNetwork(String filename, InputStream data, Computation
* and with no importer-specific parameters.
* Post processors will use the default {@link LocalComputationManager}, as defined in
* default platform config.
* Please note that the input stream must be from a simple file, not a zipped one.
*
* @param filename The name of the file to be imported.
* @param data The raw data from which the network should be loaded
Expand All @@ -630,6 +622,7 @@ public static Network loadNetwork(String filename, InputStream data) {
* and with no importer-specific parameters.
* Post processors will use the default {@link LocalComputationManager}, as defined in
* default platform config.
* Please note that the input stream must be from a simple file, not a zipped one.
*
* @param filename The name of the file to be imported.
* @param data The raw data from which the network should be loaded
Expand All @@ -640,6 +633,22 @@ public static Network loadNetwork(String filename, InputStream data, Reporter re
return loadNetwork(filename, data, LocalComputationManager.getDefault(), CONFIG.get(), null, LOADER.get(), reporter);
}

public static Network loadNetwork(ReadOnlyDataSource dataSource) {
return loadNetwork(dataSource, null);
}

public static Network loadNetwork(ReadOnlyDataSource dataSource, Properties properties) {
return loadNetwork(dataSource, properties, Reporter.NO_OP);
}

public static Network loadNetwork(ReadOnlyDataSource dataSource, Properties properties, Reporter reporter) {
Importer importer = findImporter(dataSource);
if (importer != null) {
return importer.importData(dataSource, NetworkFactory.findDefault(), properties, reporter);
}
throw new PowsyblException(UNSUPPORTED_FILE_FORMAT_OR_INVALID_FILE);
}

public static void loadNetworks(Path dir, boolean parallel, ImportersLoader loader, ComputationManager computationManager, ImportConfig config, Properties parameters, Consumer<Network> consumer, Consumer<ReadOnlyDataSource> listener, Reporter reporter) throws IOException, InterruptedException, ExecutionException {
if (!Files.isDirectory(dir)) {
throw new PowsyblException("Directory " + dir + " does not exist or is not a regular directory");
Expand Down
5 changes: 3 additions & 2 deletions iidm/iidm-converter-api/src/test/java/com/powsybl/iidm/import_/ImportersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.powsybl.commons.PowsyblException;
import com.powsybl.commons.TestUtil;
import com.powsybl.commons.datasource.DataSource;
import com.powsybl.commons.datasource.ReadOnlyDataSource;
import com.powsybl.commons.reporter.ReporterModel;
import com.powsybl.computation.ComputationManager;
import com.powsybl.iidm.AbstractConvertersTest;
Expand Down Expand Up @@ -145,8 +146,8 @@ public void setPostProcessor() {
}

@Test
public void importData() {
Network network = Importers.importData(loader, TEST_FORMAT, null, null, computationManager, importConfigMock);
public void importData() throws IOException {
Network network = Importers.loadNetwork((ReadOnlyDataSource) null);
Copy link
Member

@geofjamg geofjamg Feb 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Probably has nothing to do with this PR, but what is the purpose of this test?????
Why are we allowing a null data source and then check that with a mock importer, we have succeed to import a network?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would worth a fix but could be done in another PR, as you wish

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was to test if the importer was found in the LOADER and not the importer itself (it tests the mechanisms to find an importer).

Copy link
Member

@geofjamg geofjamg Mar 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think it was to test if the importer was found in the LOADER and not the importer itself (it tests the mechanisms to find an importer).

In any case null datasource should throw an exception even in such a test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm writing an issue about it

assertNotNull(network);
assertNotNull(network.getLoad("LOAD"));
}
Expand Down