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

Improved control over code changes #925

Merged
merged 22 commits into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
c261aa0
Added documentation of how to handle code changes.
jesper-friis Aug 12, 2024
d4a9771
Update doc/contributors_guide/code_changes.md
jesper-friis Aug 13, 2024
3437125
Apply suggestions from code review
jesper-friis Aug 13, 2024
b59fbb1
Quick update of code_changes.md
jesper-friis Aug 13, 2024
98e86b6
Merge branch 'master' into code_changes
jesper-friis Aug 13, 2024
53d6901
Merge branch 'code_changes' into behavior
jesper-friis Aug 13, 2024
e3862f2
Improved deprecation warnings and initial implementation of behavior.
jesper-friis Aug 14, 2024
e5dfbcc
Added a simple Python class for accessing behavior from Python
jesper-friis Aug 14, 2024
9949b52
Merge branch 'master' into behavior
jesper-friis Aug 14, 2024
8456d87
The python interface to behaviours.
jesper-friis Aug 14, 2024
7b406ad
Minor update
jesper-friis Aug 14, 2024
37a8c60
Added initialisation of behavior table and simplified the logics.
jesper-friis Aug 14, 2024
a1c94d3
Fixed a bug in natoi()
jesper-friis Aug 14, 2024
562baac
Fix documentation strings.
jesper-friis Aug 15, 2024
b375955
Documented environment variables DLITE_BEHAVIOR_*
jesper-friis Aug 15, 2024
b16de54
Merge branch 'master' into behavior
jesper-friis Aug 15, 2024
d124135
Apply suggestions from code review
jesper-friis Aug 16, 2024
f3fb13b
Apply suggestions from code review
jesper-friis Aug 16, 2024
0bda841
Apply suggestions from code review
jesper-friis Aug 16, 2024
71d29ee
Apply suggestions from code review
jesper-friis Aug 16, 2024
db926d4
Added more instructions to warning in dlite_behavior_get()
jesper-friis Aug 16, 2024
fad63f8
Merge branch 'master' into behavior
jesper-friis Aug 16, 2024
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: 61 additions & 0 deletions bindings/python/dlite-behavior-python.i
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* -*- Python -*- (not really, but good for syntax highlighting) */

/* Python-specific extensions to dlite-behavior.i */


%extend _DLiteBehavior {
%pythoncode %{
def asdict(self):
return {
"name": self.name,
"value": bool(self.value),
"version_added": self.version_added,
"version_new": self.version_new,
"version_remove": self.version_remove,
"description": self.description,
}

def __str__(self):
import json
return json.dumps(self.asdict(), indent=2)
%}
}


%pythoncode %{

class _Behavior():
"""A class that provides easy access for setting and getting behavior
settings.

It is used via the singleton `dlite.Behavior`.

The different behavior values can be accessed as attributes.

"""
def __getattr__(self, name):
return bool(_dlite._behavior_get(name))

def __setattr__(self, name, value):
_dlite._behavior_set(name, 1 if value else 0)

def __dir__(self):
return object.__dir__(self) + list(self.get_names())

def get_names(self):
"""Return a generator over all registered behavior names."""
for i in range(_dlite._behavior_nrecords()):
yield _dlite._behavior_recordno(i).name

def get_record(self, name):
"""Return a record with information about the given behavior."""
rec = _dlite._behavior_record(name)
if not rec:
raise DLiteKeyError(f"No such behavior record: {name}")
return rec


# A singleton for accessing behavior settings.
Behavior = _Behavior()

%}
59 changes: 59 additions & 0 deletions bindings/python/dlite-behavior.i
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/* -*- C -*- (not really, but good for syntax highlighting) */
%{
#include "dlite-behavior.h"
%}

%rename(_BehaviorRecord) _DLiteBehavior;
struct _DLiteBehavior {
%immutable;
char *name;
char *version_added;
char *version_new;
char *version_remove;
char *description;
%mutable;
int value;
};

%rename("%(strip:[dlite])s") "";


%feature("docstring", "\
Return the number of registered behaviors.
") dlite_behavior_nrecords;
size_t dlite_behavior_nrecords(void);

%feature("docstring", "\
Return a pointer to record with the given number or NULL if `n` is out of
range.
") dlite_behavior_recordno;
const struct _DLiteBehavior *dlite_behavior_recordno(size_t n);

%feature("docstring", "\
Return a pointer to the given behavior record, or NULL if `name` is
not in the behavior table.
") dlite_behavior_record;
const struct _DLiteBehavior *dlite_behavior_record(const char *name);

%feature("docstring", "\
Get value of given behavior.

Returns 1 if the behavior is on, 0 if it is off and a negative
value on error.
") dlite_behavior_get;
int dlite_behavior_get(const char *name);

%feature("docstring", "\
Enable given behavior if `value` is non-zero. Disable if `value` is zero.

Returns non-zero on error.
") dlite_behavior_set;
int dlite_behavior_set(const char *name, int value);


/* -----------------------------------
* Target language-spesific extensions
* ----------------------------------- */
#ifdef SWIGPYTHON
%include "dlite-behavior-python.i"
#endif
38 changes: 38 additions & 0 deletions bindings/python/dlite-misc-python.i
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
%pythoncode %{

import atexit
import warnings


atexit.register(_mark_python_atexit)

Expand Down Expand Up @@ -77,4 +79,40 @@ class errctl():


silent = errctl(filename="None")

# A set for keeping track of already issued deprecation warnings
_deprecation_warning_record = set()

def deprecation_warning(version_removed, descr):
"""Issues a deprecation warning.

Arguments:
version_removed: The version the deprecated feature is
expected to be finally removed.
descr: Description of the deprecated feature.

Returns non-zero on error.
"""
if descr in _deprecation_warning_record:
return
_deprecation_warning_record.add(descr)

warnings.warn(descr, DeprecationWarning, stacklevel=2)

dlite_version = get_version()
if chk_semver(version_removed) < 0:
raise SystemError(
f"argument `version_removed='{version_removed}'` must be a "
"valid semantic version number",
)
if chk_semver(dlite_version) < 0:
raise SystemError(
f"DLite version number is not semantic: {dlite_version}"
)
if cmp_semver(version_removed, dlite_version) <= 0:
raise SystemError(
"deprecated feature was supposed to be removed in version "
f"{version_removed}: {descr}"
)

%}
32 changes: 32 additions & 0 deletions bindings/python/dlite-misc.i
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,16 @@
/* Just check for errors, do nothing else. */
void errcheck(void) {}

int chk_semver(const char *version, int n) {
if (n >= 0) return strnchk_semver(version, n);
return strchk_semver(version);
}

int cmp_semver(const char *v1, const char *v2, int n) {
if (n >= 0) return strncmp_semver(v1, v2, n);
return strcmp_semver(v1, v2);
}

%}

%include <stdint.i>
Expand Down Expand Up @@ -211,6 +221,28 @@ Set error stream.
bool asbool(const char *str);


%feature("docstring", "\
Check if `version` is a valid semantic version 2.0.0 number.

By default, the whole string is checked. If `n` is non-negative, only
the initial part of `version` is checked, at most `n` characters.

Returns the length of the valid part of `version` or -1 if `version` is
not a valid semantic number.
") chk_semver;
int chk_semver(const char *version, int n=-1);

%feature("docstring", "\
Compare semantic version strings `v1` and `v2`.

Returns -1 if `v1 < v2`, 0 if `v1 == v2` and 1 if `v1 > v2`.

By default, the whole of `v1` and `v2` are compared. If `n` is non-negative,
only the initial part of `v1` and `v2` are compared, at most `n` characters.
") cmp_semver;
int cmp_semver(const char *v1, const char *v2, int n=-1);



/* -----------------------------------
* Target language-spesific extensions
Expand Down
1 change: 1 addition & 0 deletions bindings/python/dlite.i
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,4 @@
%include "dlite-collection.i"
%include "dlite-path.i"
%include "dlite-mapping.i"
%include "dlite-behavior.i"
30 changes: 30 additions & 0 deletions bindings/python/tests/test_misc.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env python
# -*- coding: utf-8 -*-
import dlite
from dlite.testutils import raises


assert dlite.get_uuid_version() == 4
Expand Down Expand Up @@ -63,3 +64,32 @@
dlite.Instance.from_location("-", "__non-existing__")
except dlite.DLiteStorageOpenError:
pass


# Test checking and comparing semantic version numbers
assert dlite.chk_semver(dlite.__version__) > 0
assert dlite.chk_semver("1.12.2-rc1.alpha+34") == 19
assert dlite.chk_semver("1.12.2-rc1.alpha+34 ") < 0
assert dlite.chk_semver("1.12.2-rc1.alpha+34 ", 6) == 6
assert dlite.chk_semver("1.12.2-rc1.alpha+34 ", 19) == 19

assert dlite.cmp_semver("1.3.11", "1.3.3") > 0
assert dlite.cmp_semver("1.3.11", "1.3.13") < 0
assert dlite.cmp_semver("1.3.11", "1.3.13", 5) == 0


# Test deprecation warnings
dlite.deprecation_warning("100.3.2", "My deprecated feature...")
dlite.deprecation_warning("100.3.2", "My deprecated feature...")
dlite.deprecation_warning("100.3.2", "My deprecated feature...")

with raises(SystemError):
dlite.deprecation_warning("0.0.1", "My deprecated feature 2...")

# Issuing the same deprecation warning a second or third time should not
# raise an exception
dlite.deprecation_warning("0.0.1", "My deprecated feature 2...")
dlite.deprecation_warning("0.0.1", "My deprecated feature 2...")

with raises(SystemError):
dlite.deprecation_warning("0.0.x", "My deprecated feature 3...")
30 changes: 18 additions & 12 deletions doc/contributors_guide/code_changes.md
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
Code changes
============
Although that DLite haven't reached version 1.0.0, it strives to be stable and not introduce unnecessary backward-incompatible changes.
Although that DLite has not reached version 1.0.0, the intention is for it to be stable and not introduce unnecessary backward-incompatible changes.


Versioning
----------
Before reaching v 1.0.0, DLite follows the following versioning rules that are well-known from Python:
Before reaching version 1.0.0, DLite follows the following versioning rules that are well-known from Python:

* **patch release**: Only backward-compatible changes.
- Addition of new backward-compatible features.
- New deprecation warnings can be added.
jesper-friis marked this conversation as resolved.
Show resolved Hide resolved
- Bug fixes.

* **minor release**: Backward-incompatible changes.
- Addition of new backward-incompatible features.
Expand All @@ -21,11 +22,10 @@ After version 1.0.0, DLite should strictly follow the [semantic versioning] rule

Feature deprecation
-------------------
In Python, use `dlite.deprecation_warning()` (function to be added) to mark deprecation warnings.
In Python, use `dlite.deprecation_warning()` to mark deprecation warnings.
This function is a simple wrapper around `warnings.warn(msg, DeprecationWarning, stacklevel=2)` with the release version when the feature is planned to be removed as an additional argument.

In C, use the `deprecation_warning()` function (to be added) to mark a
deprecation warning.
In C, use the `dlite_deprecation_warning()` function to mark a deprecation warning.

The idea with these functions is to make it easy to grep for deprecation warnings that should be remove in a new backward-incompatible release.
The following command can e.g. be used to find all deprecation warnings:
Expand All @@ -43,7 +43,7 @@ Adding new arguments to an existing function or new methods to existing classes
**Note**, new (positional) arguments should be added to the end of the argument list, such that existing code using positional arguments will be unaffected by the change.

### New behavior
All changes in behavior should be considered backward-incompatible.
All changes in behavior in existing code should be considered backward-incompatible.

Where it make sense, DLite will try to optionally keep the new/old behaviour over some releases in order for users to adapt to the new behavior.

Expand All @@ -53,23 +53,29 @@ Each optional new behavior should have a:
- release number for when the behavior was introduced (keeping old behavior as default)
- release number for when the new behavior should be the default (managed automatically)
- expected release number for when the old behavior should be removed (require developer effort)
- value: True means that the behavior is enabled.

All behaviors are described in a single source file `src/dlite-behavior.c`.
All behavior changes will be described in a single source file `src/dlite-behavior.c`.

Behavior can be selected in several ways:
Whether to enable a behavior can be configured in several ways:

- **programmatically from Python**:

>>> from dlite.behavior import Behavior
>>> Behavior.<NAME> = True # true means use the new behavior
>>> dlite.Behavior.<NAME> = True

- **programmatically from C**:

dlite_behavior("<NAME>", 1);
dlite_behavior_set("<NAME>", 1);

- **via environment variables**:

export DLITE_BEHAVIOR_<NAME>
export DLITE_BEHAVIOR_<NAME>=1

The empty string or any of the following values will enable the behavior:
"true", ".true.", "on", "yes", 1

Any of the following values will disable the behavior:
"false", ".false.", "off", "no", 0

A warning will automatically be issued if a behavior is not selected explicitly.

Expand Down
8 changes: 8 additions & 0 deletions doc/user_guide/environment_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ DLite-specific environment variables
- **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.

- **DLITE_BEHAVIOR_<name>**: Enables/disables the behavior `<name>`.

The empty string or any of the following values will enable the behavior:
"true", ".true.", "on", "yes", 1

Any of the following values will disable the behavior:
"false", ".false.", "off", "no", 0


### Specific paths
These environment variables can be used to provide additional search
Expand Down
1 change: 1 addition & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ set(sources
dlite-getlicense.c
dlite-json.c
dlite-bson.c
dlite-behavior.c
getuuid.c
pathshash.c
triple.c
Expand Down
Loading