add more tests

don't use get_item_indices
version bump

CMakeLists.txt

@@ -34,7 +34,7 @@ endif()
 # set the project name
 project(
     libcifpp
-    VERSION 10.0.2
+    VERSION 10.0.4
     LANGUAGES CXX C)
 
 list(PREPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")

changelog

@@ -1,3 +1,13 @@
+Version 10.0.4
+- Fixed find_by_value in the index of a category,
+  avoid swapping columns in the search keys
+
+Version 10.0.3
+- Clear pdbx_nonpoly_scheme before filling it in reconstruction
+- Changed handling of numbers with a preceding plus character,
+  these are now stored as strings to avoid inadvertently
+  mutilating phone numbers.
+
 Version 10.0.2
 - Fixed regression in reconstruction introduced in 10.0.1
 - Fixed symmetry operations

include/cif++/item.hpp

@@ -331,8 +331,11 @@ class item_value
 			case TEXT:
 			{
 				auto sv = m_data.sv();
+				auto sp = sv.data();
+				if (*sp == '+')
+					++sp;
 				int64_t v;
-				auto &&[ptr, ec] = from_chars(sv.data(), sv.data() + sv.length(), v);
+				auto &&[ptr, ec] = from_chars(sp, sv.data() + sv.length(), v);
 				if (ec != std::errc{})
 					throw std::system_error(std::make_error_code(ec));
 				if (ptr != sv.data() + sv.length())
@@ -361,8 +364,11 @@ class item_value
 			case TEXT:
 			{
 				auto sv = m_data.sv();
+				auto sp = sv.data();
+				if (*sp == '+')
+					++sp;
 				double v;
-				auto &&[ptr, ec] = from_chars(sv.data(), sv.data() + sv.length(), v);
+				auto &&[ptr, ec] = from_chars(sp, sv.data() + sv.length(), v);
 				if (ec != std::errc{})
 					throw std::system_error(std::make_error_code(ec));
 				if (ptr != sv.data() + sv.length())

src/category.cpp

@@ -118,6 +118,7 @@ class row_comparator
 
 		for (const auto &[k, f] : m_comparator)
 		{
+			assert(cat.get_item_name(k) == ai->name);
 			d = f(ai->value, rhb[k].value());
 
 			if (d != 0)
@@ -363,10 +364,9 @@ row *category_index::find_by_value(const category &cat, const category::key_type
 	// sort the values in k first
 
 	category::key_type k2;
-	for (auto &f : cat.key_item_indices())
+	auto cv = cat.get_cat_validator();
+	for (auto &fld : cv->m_keys)
 	{
-		auto fld = cat.get_item_name(f);
-
 		auto ki = std::ranges::find_if(k, [&fld](auto &i)
 			{ return i.name == fld; });
 		if (ki == k.end())
@@ -759,14 +759,10 @@ void category::set_validator(const validator *v, datablock &db)
 				if (not v.is_number())
 				{
 					// Try cast the value to a number and throw in case of failure
-					try
-					{
+					if (auto sv = v.sv(); sv.find_first_of(".eE") == std::string_view::npos)
 						v.cast_to_int();
-					}
-					catch (...)
-					{
+					else
 						v.cast_to_float();
-					}
 				}
 			}
 		}

src/condition.cpp

@@ -129,9 +129,9 @@ namespace detail
 			m_item_ix = *ix;
 			m_icase = is_item_type_uchar(c, m_item_name);
 
-			if (c.get_cat_validator() != nullptr and
-				c.key_item_indices().contains(m_item_ix) and
-				c.key_item_indices().size() == 1)
+			if (auto cv = c.get_cat_validator();
+				cv != nullptr and cv->m_keys.size() == 1 and
+				cv->m_keys.front() == m_item_name)
 			{
 				m_single_hit = c[{ { m_item_name, m_value } }];
 			}

src/item.cpp

@@ -239,7 +239,12 @@ void item_value::cast_to_int()
 		{
 			auto s = sv();
 			int64_t v;
-			auto [ptr, ec] = cif::from_chars(s.data(), s.data() + s.size(), v);
+
+			auto sp = s.data();
+			if (*sp == '+')
+				++sp;
+
+			auto [ptr, ec] = cif::from_chars(sp, s.data() + s.size(), v);
 			if (ec != std::errc{})
 				throw std::system_error(std::make_error_code(ec), "attempt to cast value to integer failed");
 			if (ptr != s.data() + s.size())

src/parser.cpp

@@ -25,6 +25,7 @@
  */
 
 #include "cif++/cif++.hpp"
+#include "cif++/utilities.hpp"
 
 #include <cassert>
 #include <cctype>
@@ -635,15 +636,28 @@ sac_parser::CIFToken sac_parser::get_next_token()
 
 	if (result == CIFToken::VALUE_NUMERIC_INTEGER)
 	{
+		// Avoid interpreting phone numbers as integers, TODO: check if this is an issue
 		auto [ptr, ec] = from_chars(m_token_buffer.data(), m_token_buffer.data() + m_token_buffer.size(), m_token_value_int);
 		if (ec != std::errc{})
-			error("Invalid integer value: " + std::make_error_code(ec).message());
+		{
+			if (cif::VERBOSE > 0)
+				std::clog << "Invalid integer value: " << std::make_error_code(ec).message() << '\n';
+
+			result = CIFToken::VALUE_CHARSTRING;
+			m_token_value = std::string_view(m_token_buffer.data(), m_token_buffer.size());
+		}
 	}
 	else if (result == CIFToken::VALUE_NUMERIC_FLOAT)
 	{
 		auto [ptr, ec] = from_chars(m_token_buffer.data(), m_token_buffer.data() + m_token_buffer.size(), m_token_value_float);
 		if (ec != std::errc{})
-			error("Invalid integer value: " + std::make_error_code(ec).message());
+		{
+			if (cif::VERBOSE > 0)
+				std::clog << "Invalid floating point value: " << std::make_error_code(ec).message() << '\n';
+
+			result = CIFToken::VALUE_CHARSTRING;
+			m_token_value = std::string_view(m_token_buffer.data(), m_token_buffer.size());
+		}
 	}
 
 	return result;

src/pdb/reconstruct.cpp

@@ -25,6 +25,7 @@
  */
 
 #include "cif++/cif++.hpp"
+#include "cif++/validate.hpp"
 
 #include <algorithm>
 #include <cstddef>
@@ -1340,6 +1341,8 @@ void createPdbxNonpolyScheme(datablock &db)
 	auto &pdbx_nonpoly_scheme = db["pdbx_nonpoly_scheme"];
 	auto &atom_site = db["atom_site"];
 
+	pdbx_nonpoly_scheme.clear();
+
 	for (const auto &[entity_id, comp_id] : pdbx_entity_nonpoly.rows<std::string, std::string>("entity_id", "comp_id"))
 	{
 		for (int ndb_nr = 1; auto row : atom_site.find("label_entity_id"_key == entity_id and "label_comp_id"_key == comp_id))
@@ -1660,7 +1663,7 @@ bool reconstruct_pdbx(file &file, const validator &validator)
 				if (not iv)
 				{
 					// Drop this item
-					cat.remove_item(item_name);
+					// cat.remove_item(item_name);
 					continue;
 				}
 
@@ -1689,6 +1692,13 @@ bool reconstruct_pdbx(file &file, const validator &validator)
 								continue;
 						}
 
+						if (ec == cif::make_error_code(cif::validation_error::value_is_not_in_enumeration_list))
+						{
+							if (VERBOSE > 0)
+								std::clog << "Value (" << std::quoted(row[ix].str()) << ") for item " << item_name << " in category " << cat.name() << " is not valid since it is not in the list of allowed values\n";
+							continue;
+						}
+
 						if (VERBOSE > 0)
 							std::clog << "Replacing value (" << std::quoted(row[ix].str()) << ") for item " << item_name << " in category " << cat.name() << " since it does not validate: " << ec.message() << "\n";
 

test/unit-v2-test.cpp

@@ -3664,5 +3664,63 @@ HETATM 2 O O . HOH A 1 . ? 10.518 -1.781 0 1 37.22 ? O HOH 2 D 1
 )");
 }
 
+// --------------------------------------------------------------------
 
+TEST_CASE("number-test-1")
+{
+	auto data = R"(data_test
+_pdbx_contact_author.id      1
+_pdbx_contact_author.name_mi +98765432109
+)"_cf;
 
+	auto &db = data.front();
+	db.load_dictionary("mmcif_pdbx.dic");
+
+	auto r = db["pdbx_contact_author"].front();
+	CHECK(r["name_mi"].str() == "+98765432109");
+	CHECK(r["name_mi"].get<int64_t>() == 98765432109);
+	CHECK(r["name_mi"].get<double>() == 98765432109.0);
+}
+
+TEST_CASE("number-test-2")
+{
+	auto data = R"(data_test
+_pdbx_contact_author.id      1
+_pdbx_contact_author.name_mi '+98765432109'
+)"_cf;
+
+	auto &db = data.front();
+	db.load_dictionary("mmcif_pdbx.dic");
+
+	auto r = db["pdbx_contact_author"].front();
+	CHECK(r["name_mi"].str() == "+98765432109");
+	CHECK(r["name_mi"].get<int64_t>() == 98765432109);
+	CHECK(r["name_mi"].get<double>() == 98765432109.0);
+}// --------------------------------------------------------------------
+
+TEST_CASE("q-1")
+{
+	auto data = R"(data_test
+_test.s
+;1234567890
+1234567890
+;
+	)"_cf;
+
+	auto r = data.front()["test"].find(cif::key("s") == "1234567890\n1234567890");
+	CHECK(r.size() == 1);
+}
+
+TEST_CASE("large-int-1")
+{
+	auto data = R"(data_test
+_entry.id     82E4475FF8B27F36
+)"_cf;
+
+	auto &db = data.front();
+	db.load_dictionary("mmcif_pdbx.dic");
+
+	auto r = db["entry"].front();
+
+	CHECK(r["id"].get<std::string>() == "82E4475FF8B27F36");
+}