Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions include/openPMD/Error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,17 @@ namespace error
OperationUnsupportedInBackend(
std::string backend_in, std::string what );
};

/**
* @brief The API was used in an illegal way.
*
* Example: File-based iteration encoding is selected without specifying an
* expansion pattern.
*/
class WrongAPIUsage : public Error
{
public:
WrongAPIUsage( std::string what );
};
}
}
4 changes: 3 additions & 1 deletion include/openPMD/Series.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class SeriesData : public AttributableData
std::string m_name;
std::string m_filenamePrefix;
std::string m_filenamePostfix;
int m_filenamePadding;
int m_filenamePadding = -1;
IterationEncoding m_iterationEncoding{};
Format m_format;
/**
Expand Down Expand Up @@ -346,6 +346,8 @@ class SeriesInterface : public AttributableInterface
} }

std::unique_ptr< ParsedInput > parseInput(std::string);
bool hasExpansionPattern( std::string filenameWithExtension );
bool reparseExpansionPattern( std::string filenameWithExtension );
void init(std::shared_ptr< AbstractIOHandler >, std::unique_ptr< ParsedInput >);
void initDefaults( IterationEncoding );
/**
Expand Down
5 changes: 5 additions & 0 deletions src/Error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,10 @@ namespace error
, backend{ std::move( backend_in ) }
{
}

WrongAPIUsage::WrongAPIUsage( std::string what )
: Error( "Wrong API usage: " + what )
{
}
}
}
63 changes: 58 additions & 5 deletions src/Series.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "openPMD/IO/AbstractIOHandler.hpp"
#include "openPMD/IO/AbstractIOHandlerHelper.hpp"
#include "openPMD/IO/Format.hpp"
#include "openPMD/Error.hpp"
#include "openPMD/ReadIterations.hpp"
#include "openPMD/Series.hpp"
#include "openPMD/version.hpp"
Expand Down Expand Up @@ -87,7 +88,7 @@ struct SeriesInterface::ParsedInput
IterationEncoding iterationEncoding;
std::string filenamePrefix;
std::string filenamePostfix;
int filenamePadding;
int filenamePadding = -1;
}; //ParsedInput

SeriesInterface::SeriesInterface(
Expand Down Expand Up @@ -280,8 +281,20 @@ SeriesInterface::setIterationEncoding(IterationEncoding ie)
switch( ie )
{
case IterationEncoding::fileBased:
setIterationFormat(series.m_name);
setAttribute("iterationEncoding", std::string("fileBased"));
setIterationFormat( series.m_name );
setAttribute( "iterationEncoding", std::string( "fileBased" ) );
// This checks that the name contains the expansion pattern
// (e.g. %T) and parses it
if( series.m_filenamePadding < 0 )
{
if( !reparseExpansionPattern( series.m_name ) )
{
throw error::WrongAPIUsage(
"For fileBased formats the iteration expansion pattern "
"%T must "
"be included in the file name" );
}
}
break;
case IterationEncoding::groupBased:
setIterationFormat(BASEPATH);
Expand Down Expand Up @@ -330,8 +343,27 @@ SeriesInterface::setName(std::string const& n)
if( written() )
throw std::runtime_error("A files name can not (yet) be changed after it has been written.");

if( series.m_iterationEncoding == IterationEncoding::fileBased && !auxiliary::contains(series.m_name, "%T") )
throw std::runtime_error("For fileBased formats the iteration regex %T must be included in the file name");
if( series.m_iterationEncoding == IterationEncoding::fileBased )
{
// If the filename specifies an expansion pattern, set it.
// If not, check if one is already active.
// Our filename parser expects an extension, so just add any and ignore
// the result for that
if( hasExpansionPattern( n + ".json" ) )
{
reparseExpansionPattern( n + ".json" );
}
else if( series.m_filenamePadding < 0 )
{
throw error::WrongAPIUsage(
"For fileBased formats the iteration expansion pattern %T must "
"be included in the file name" );
}
else
{
// no-op, keep the old pattern and set the new name
}
}

series.m_name = n;
dirty() = true;
Expand Down Expand Up @@ -418,6 +450,27 @@ SeriesInterface::parseInput(std::string filepath)
return input;
}

bool SeriesInterface::hasExpansionPattern( std::string filenameWithExtension )
{
auto input = parseInput( std::move( filenameWithExtension ) );
return input->iterationEncoding == IterationEncoding::fileBased;
}

bool SeriesInterface::reparseExpansionPattern(
std::string filenameWithExtension )
{
auto input = parseInput( std::move( filenameWithExtension ) );
if( input->iterationEncoding != IterationEncoding::fileBased )
{
return false;
}
auto & series = get();
series.m_filenamePrefix = input->filenamePrefix;
series.m_filenamePostfix = input->filenamePostfix;
series.m_filenamePadding = input->filenamePadding;
return true;
}

void SeriesInterface::init(
std::shared_ptr< AbstractIOHandler > ioHandler,
std::unique_ptr< SeriesInterface::ParsedInput > input )
Expand Down
2 changes: 2 additions & 0 deletions src/binding/python/Error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ void init_Error( py::module & m )
auto & baseError = py::register_exception< Error >( m, "Error" );
py::register_exception< error::OperationUnsupportedInBackend >(
m, "ErrorOperationUnsupportedInBackend", baseError );
py::register_exception< error::WrongAPIUsage >(
m, "ErrorWrongAPIUsage", baseError );

#ifndef NDEBUG
m.def( "test_throw", []( std::string description ) {
Expand Down
4 changes: 2 additions & 2 deletions test/CoreTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -688,14 +688,14 @@ TEST_CASE( "structure_test", "[core]" )

TEST_CASE( "wrapper_test", "[core]" )
{
Series o = Series("./new_openpmd_output.json", Access::CREATE);
Series o = Series("./new_openpmd_output_%T.json", Access::CREATE);

o.setOpenPMDextension(42);
o.setIterationEncoding(IterationEncoding::fileBased);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fun fact: This is exactly the kind of wrong usage that #1048 reports. Since this PR adds sanity-checks to catch such behavior, I'm changing this Series to be file-based now.

Series copy = o;
REQUIRE(copy.openPMDextension() == 42);
REQUIRE(copy.iterationEncoding() == IterationEncoding::fileBased);
REQUIRE(copy.name() == "new_openpmd_output");
REQUIRE(copy.name() == "new_openpmd_output_%T");
copy.setOpenPMD("1.2.0");
copy.setIterationEncoding(IterationEncoding::groupBased);
copy.setName("other_name");
Expand Down
49 changes: 49 additions & 0 deletions test/SerialIOTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4566,3 +4566,52 @@ TEST_CASE( "no_explicit_flush", "[serial]" )
no_explicit_flush( "../samples/no_explicit_flush." + t );
}
}

TEST_CASE( "late_setting_of_iterationencoding", "[serial]" )
{
{
::openPMD::Series series = ::openPMD::Series(
"../samples/error.json", ::openPMD::Access::CREATE );
series.iterations[ 10 ];
REQUIRE_THROWS_WITH(
series.setIterationEncoding(
::openPMD::IterationEncoding::fileBased ),
Catch::Equals( "Wrong API usage: For fileBased formats the "
"iteration expansion pattern %T must "
"be included in the file name" ) );
series.flush();
}
{
::openPMD::Series series = ::openPMD::Series(
"../samples/asdf_%T.json",
::openPMD::Access::CREATE );
series.iterations[ 10 ];
series.setName(
"change_name_%T" );
series.flush();
}
{
::openPMD::Series series = ::openPMD::Series(
"../samples/change_name_keep_filename_%T.json",
::openPMD::Access::CREATE );
series.iterations[ 10 ];
series.setName(
"expansion_pattern_was_specified_previously_filename_will_stay" );
series.flush();
}
{
::openPMD::Series series = ::openPMD::Series(
"../samples/asdf.json", ::openPMD::Access::CREATE );
series.iterations[ 10 ];
series.setName( "change_name_and_encoding_%T" );
series.setIterationEncoding( IterationEncoding::fileBased );
series.flush();
}

REQUIRE( auxiliary::file_exists(
"../samples/change_name_10.json" ) );
REQUIRE( auxiliary::file_exists(
"../samples/change_name_keep_filename_10.json" ) );
REQUIRE( auxiliary::file_exists(
"../samples/change_name_and_encoding_10.json" ) );
}