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.

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

Applied general fixes from PR 667 #720

Merged
merged 3 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 3 additions & 2 deletions bindings/python/tests/test_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

thisdir = Path(__file__).resolve().parent
rootdir = thisdir.parent.parent.parent
outdir = thisdir / "output"


class Person:
Expand All @@ -29,12 +30,12 @@ def __repr__(self):

print("-- create: person2")
person2 = ExPerson("Jack Daniel", 42, ["distilling", "tasting"])
person2.dlite_inst.save("json", "persons.json", "mode=w")
person2.dlite_inst.save("json", f"{outdir}/persons.json", "mode=w")

# Print json-representation of person2 using dlite
print(person2.dlite_inst.asjson(indent=2))

inst = dlite.Instance.from_url("json://persons.json")
inst = dlite.Instance.from_url(f"json://{outdir}/persons.json")
person3 = dlite.instancefactory(Person, inst)

person4 = dlite.objectfactory(person1, meta=person2.dlite_meta)
Expand Down
3 changes: 2 additions & 1 deletion bindings/python/tests/test_property_mappings.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import numpy as np

try:
from tripper import Triplestore
import rdflib
from pint import Quantity
from tripper import Triplestore
except ImportError as exc:
sys.exit(44) # exit code marking the test to be skipped

Expand Down
2 changes: 1 addition & 1 deletion bindings/python/tests/test_pydantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class TransformationStatus(BaseModel):
assert inst.startTime == int(now - 3000)
utc = timezone(timedelta(hours=0))
dt = datetime.fromtimestamp(now - 600).astimezone(utc)
assert inst.finishTime == str(dt)
assert inst.finishTime.split("+")[0] == str(dt).split("+")[0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please comment why

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done



# ==============================================================
Expand Down
15 changes: 8 additions & 7 deletions bindings/python/tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@


thisdir = os.path.abspath(os.path.dirname(__file__))
outdir = f"{thisdir}/output"

url = "json://" + thisdir + "/MyEntity.json"

Expand All @@ -35,26 +36,26 @@
inst["a-bool-array"] = True, False

# Test Storage.save()
with dlite.Storage("json", "tmp.json", "mode=w") as s:
with dlite.Storage("json", f"{outdir}/tmp.json", "mode=w") as s:
s.save(inst)

# Test query


# Test json
print("--- testing json")
myentity.save("json://myentity.json?mode=w")
inst.save("json://inst.json?mode=w")
myentity.save(f"json://{outdir}/myentity.json?mode=w")
inst.save(f"json://{outdir}/inst.json?mode=w")
del inst
inst = dlite.Instance.from_url(f"json://{thisdir}/inst.json#my-data")
inst = dlite.Instance.from_url(f"json://{outdir}/inst.json#my-data")


# Test yaml
if HAVE_YAML:
print('--- testing yaml')
inst.save('yaml://inst.yaml?mode=w')
inst.save(f"yaml://{outdir}/inst.yaml?mode=w")
del inst
inst = dlite.Instance.from_url("yaml://inst.yaml#my-data")
inst = dlite.Instance.from_url(f"yaml://{outdir}/inst.yaml#my-data")

# test help()
expected = """\
Expand All @@ -73,7 +74,7 @@
- `single`: Whether the input is assumed to be in single-entity form.
If "auto" (default) the form will be inferred automatically.
"""
s = dlite.Storage("yaml", "inst.yaml", options="mode=a")
s = dlite.Storage("yaml", f"{outdir}/inst.yaml", options="mode=a")
assert s.help().strip() == expected.strip()

# Test delete()
Expand Down
42 changes: 41 additions & 1 deletion cmake/valgrind-python.supp
Copy link
Collaborator

Choose a reason for hiding this comment

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

Describe the purpose of this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,47 @@
fun:_PyEval_EvalFrameDefault
}

{
Ignore conditional jump depends on uninitialised variable
Memcheck:Cond
fun:__wcsncpy_avx2
...
fun:_PyPathConfig_Calculate
...
fun:_PyPathConfig_Init
...
fun:_PyCoreConfig_Read
...
fun:_Py_UnixMain
}

{
Invalid read in main (py39)
Memcheck:Addr32
fun:__wcsncpy_avx2
fun:_PyPathConfig_Calculate
fun:_PyPathConfig_Init
fun:_PyCoreConfig_Read
...
fun:_Py_UnixMain
}

{
Invalid read in main (py311)
Memcheck:Addr32
fun:__wcsncpy_avx2
...
fun:_PyObject_MakeTpCall
fun:_PyEval_EvalFrameDefault
...
fun:PyEval_EvalCode
...
fun:Py_InitializeFromConfig
...
fun:Py_BytesMain
}


# ------------------------------------
# Python plugins
# ------------------------------------
Expand Down Expand Up @@ -967,7 +1008,6 @@
}



# ------------------------------------------
# valgrind suppressions generated from
# ts.log
Expand Down
1 change: 1 addition & 0 deletions doc/contributors_guide/tips_and_tricks.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Debugging tests failing inside docker on GitHub

3. To list all manylinux images for Python 3.7, do

cd dlite # Root of DLite source directory
CIBW_MANYLINUX_X86_64_IMAGE=ghcr.io/sintef/dlite-python-manylinux2014_x86_64:latest \
CIBW_BUILD=cp37-manylinux_* \
python -m cibuildwheel \
Expand Down
3 changes: 3 additions & 0 deletions doc/user_guide/environment_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ DLite-specific environment variables
lost. But, if `DLITE_PYDEBUG` is defined, a Python error message will be
written to standard error.

- **DLITE_ATEXIT_FREE**: Free memory at exit. This might be useful to avoid
getting false positive when tracking down memory leaks with tools like valgrind.


### Specific paths
These environment variables can be used to provide additional search
Expand Down
4 changes: 3 additions & 1 deletion src/dlite-storage-plugins.c
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment why

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To avoid memory leak by calling fu_endmatch() when we are done with the iterator.

To not polute the code with unnecessary comments, I prefer not to add this comment to the code since it is already documented in fu_startmatch() that the returned iterator must be cleaned up with fu_endmatch().

Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ const DLiteStoragePlugin *dlite_storage_plugin_get(const char *name)
r = asnpprintf(&buf, &size, m, " - %s\n", p);
if (r >= 0) m += r;
}
fu_endmatch(iter);

if ((failed_paths = dlite_python_storage_failed_paths())) {
r = asnpprintf(&buf, &size, m,
Expand All @@ -178,15 +179,16 @@ const DLiteStoragePlugin *dlite_storage_plugin_get(const char *name)
}
}

#endif
if (n <= 1)
m += asnpprintf(&buf, &size, m,
" Are the required Python packages installed or %s\n"
" DLITE_STORAGE_PLUGIN_DIRS or "
"DLITE_PYTHON_STORAGE_PLUGIN_DIRS\n"
" environment variables set?", submsg);
errx(1, "%s", buf);
#endif
free(buf);

}
return NULL;
}
Expand Down
4 changes: 2 additions & 2 deletions storages/json/dlite-json-storage.c
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment on in commit

Copy link
Collaborator Author

@jesper-friis jesper-friis Dec 2, 2023

Choose a reason for hiding this comment

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

Changed name of variable from "state" to "globals" to be consistent with variable naming when get_dlite_storage_plugin_api() is called in the rest of the code.

I also think this comment belongs better here than in the code.

Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,9 @@ static DLiteStoragePlugin dlite_json_plugin = {


DSL_EXPORT const DLiteStoragePlugin *
get_dlite_storage_plugin_api(void *state, int *iter)
get_dlite_storage_plugin_api(void *globals, int *iter)
{
UNUSED(iter);
dlite_globals_set(state);
dlite_globals_set(globals);
return &dlite_json_plugin;
}
2 changes: 1 addition & 1 deletion storages/python/python-storage-plugins/mongodb.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def load(self, id):
document = self.collection.find_one({"uuid": uuid})
if not document:
raise IOError(
f"No instance with {uuid=} in MongoDB database "
f"No instance with uuid={uuid} in MongoDB database "
f'"{self.collection.database.name}" and collection '
f'"{self.collection.name}"'
)
Expand Down
2 changes: 2 additions & 0 deletions storages/python/tests-c/test_bson_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ MU_TEST(test_load)
mu_assert_string_eq(json_str2, bson_str2);
free(json_str1);
free(json_str2);
free(bson_str1);
free(bson_str2);

stat = dlite_storage_close(s);
mu_assert_int_eq(0, stat);
Expand Down
1 change: 1 addition & 0 deletions storages/python/tests-python/test_mongodb_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import dlite

try:
import pymongo
import mongomock
except ImportError:
sys.exit(44) # skip test
Expand Down
Loading