Skip to content

Fix #731, Improved error message when reading a data that is not defined in specification tree #732

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

Open
jmorice91 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix #731, Improved error message when reading a data that is not defined in specification tree #732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b627324
fix #731
jmorice91 Jul 27, 2026
c36258a
fix #731, adding exposed data name in the message
jmorice91 Jul 30, 2026
f87af67
Add review comment
jmorice91 Aug 7, 2026
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ and this project adheres to
### For users

#### Added
* Improved error message in type check in Decl'NetCDF when reading scalar variable from file
[#731]https://github.com/pdidev/pdi/issues/731

#### Changed

Expand Down Expand Up @@ -99,7 +101,6 @@ and this project adheres to
* Fixed the build failure of a test in 1.11.1



## [1.11.1] - 2026-06-23

### For users
Expand Down
3 changes: 3 additions & 0 deletions pdi/include/pdi/scalar_datatype.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ class PDI_EXPORT Scalar_datatype: public Datatype
std::function<void(void*) > destroy,
const Attributes_map& attributes = {}
);

/// return if the scalar datatype is a nulltype
bool is_nulltype() const;
Comment thread
jbigot marked this conversation as resolved.
};

const auto UNDEF_TYPE = Scalar_datatype::make(Scalar_kind::UNKNOWN, 0);
Expand Down
5 changes: 5 additions & 0 deletions pdi/src/scalar_datatype.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,9 @@ shared_ptr<Scalar_datatype> Scalar_datatype::make(
return make_shared<Shared_enabler>(kind, size, align, dense_size, copy, destroy, attributes);
}

bool Scalar_datatype::is_nulltype() const
{
return nulltype(*this);
}

Comment thread
jbigot marked this conversation as resolved.
} // namespace PDI
4 changes: 2 additions & 2 deletions plugins/decl_netcdf/dnc_file_context.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ void Dnc_file_context::execute(const std::string& desc_name, PDI::Ref ref)
nc_file.read_variable(*variable);

// execute read
nc_file.get_variable(*variable, read_it->second, ref);
nc_file.get_variable(*variable, read_it->second, ref, desc_name);
}

auto size_it = m_sizeof.find(desc_name);
Expand Down Expand Up @@ -352,7 +352,7 @@ void Dnc_file_context::execute()
i = 0;
for (auto&& read: m_read) {
Dnc_variable* variable = variables_to_get[i]; // order of loop iteration is the same as was on define loop
nc_file->get_variable(*variable, read.second, m_ctx.desc(read.first).ref());
nc_file->get_variable(*variable, read.second, m_ctx.desc(read.first).ref(), read.first);
i++;
}
}
Expand Down
22 changes: 18 additions & 4 deletions plugins/decl_netcdf/dnc_netcdf_file.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ void Dnc_netcdf_file::put_variable(const Dnc_variable& variable, const Dnc_io& w
m_ctx.logger().trace("Variable `{}' written", variable_name);
}

void Dnc_netcdf_file::get_variable(const Dnc_variable& variable, const Dnc_io& read, PDI::Ref_w ref_w)
void Dnc_netcdf_file::get_variable(const Dnc_variable& variable, const Dnc_io& read, PDI::Ref_w ref_w, const std::string& ref_name)
{
if (!ref_w) {
throw PDI::Permission_error{"Decl_netcdf plugin: Cannot read `{}'. Need write access to read it from file", variable.path()};
Expand Down Expand Up @@ -690,12 +690,12 @@ void Dnc_netcdf_file::get_variable(const Dnc_variable& variable, const Dnc_io& r
}
nc_id var_id = var_it->second;

// read variable and check for scalar type match
if (auto&& scalar_type = std::dynamic_pointer_cast<const PDI::Scalar_datatype>(ref_w.type())) {
if (auto&& scalar_type = std::dynamic_pointer_cast<const PDI::Scalar_datatype>(ref_w.type()->evaluate(m_ctx))) {
nc_type var_nc_type;
size_t var_nc_type_size;
nc_try(nc_inq_vartype(src_id, var_id, &var_nc_type), "cannot get type of `{}' from file", variable.path());
nc_try(nc_inq_type(0, var_nc_type, NULL, &var_nc_type_size), "can not inquire the size of `{}'", var_nc_type);

if (scalar_type->kind() == PDI::Scalar_kind::SIGNED) {
switch (var_nc_type) {
case NC_BYTE:
Expand Down Expand Up @@ -749,8 +749,22 @@ void Dnc_netcdf_file::get_variable(const Dnc_variable& variable, const Dnc_io& r
scalar_type->datasize()
};
}
} else if ((*scalar_type) == (*PDI::UNDEF_TYPE)) {
throw PDI::Type_error{
"Can not read `{}' : Invalid type in Decl_netcdf plugin: "
"The exposed data `{}' is not defined in yaml (meta)data section.",
variable_name,
ref_name
};

@jbigot jbigot Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you could do a else if ( (*scalar_type ) == UNDEF_TYPE ) { ... } to handle this case as you were doing before and finish with a else { ... } where you throw an Impl_error for unimplemented scalar kinds

@jmorice91 jmorice91 Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In this list of else, we are in a case of scalar_datatype. The last else correspond to the unknown case and we return a Type_error as the function nc_scalar_type in this file line 114.

} else {
throw PDI::Type_error{"Can not read `{}' : buffer has unknown type", variable_name};
throw PDI::Type_error{
"Can not read `{}' : Invalid type in Decl_netcdf plugin: "
"The exposed data `{}' is defined with an unsupported unknown"
" scalar datatype: {}.",
variable_name,
ref_name,
scalar_type->debug_string()
};
}
}

Expand Down
3 changes: 2 additions & 1 deletion plugins/decl_netcdf/dnc_netcdf_file.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,9 @@ class Dnc_netcdf_file
* \param variable variable to get
* \param write Dnc_io that deteremines the read operation
* \param ref_w reference where the data will be written
* \param ref_name name of the reference ref_w
*/
void get_variable(const Dnc_variable& variable, const Dnc_io& read, PDI::Ref_w ref_w);
void get_variable(const Dnc_variable& variable, const Dnc_io& read, PDI::Ref_w ref_w, const std::string& ref_name);

/** Gets variable from the file
*
Expand Down
2 changes: 1 addition & 1 deletion plugins/decl_netcdf/tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ endif()


add_executable(decl_netcdf_tests decl_netcdf_tests.cxx)
target_link_libraries(decl_netcdf_tests PDI::PDI_plugins Threads::Threads GTest::gtest GTest::gtest_main)
target_link_libraries(decl_netcdf_tests PDI::PDI_plugins Threads::Threads GTest::gmock GTest::gtest GTest::gtest_main)
target_compile_features(decl_netcdf_tests PUBLIC cxx_std_20)
gtest_discover_tests(decl_netcdf_tests)

Expand Down
Loading
Loading