Add support for GSF v3.09 #125

lmarsden · 2021-02-23T18:34:46Z

Introduces a pattern for supporting future new versions of GSF.
Default supported version in the top level gsfpy namespace remains as v3.08 for backwards compatibility.
Provides option for changing default version in top level namespace via a DEFAULT_GSF_VERSION environment variable.
Provides version-specific gsfpy3_08 and gsfpy3_09 namespaces as alternative to defining the default GSF version via environment.

* Also introduces a pattern for supporting future new versions of GSF. * Default supported version in the top level gsfpy namespace remains as v3.08 for backwards compatibility. * Provides option for changing default version in top level namespace via a DEFAULT_GSF_VERSION environment variable. * Provides version-specific gsfpy3_08 and gsfpy3_09 namespaces as alternative to defining the default GSF version at the top level.

* Note that the top-level gsfpy namespace is not included in coverage reporting as it simply acts as a switch between gsfpy3_08 and gsfpy3_09.

* For improved backward compatibility it must be possible to do imports of the form 'from gsfpy.enums import ...'

* In order that imports of the form 'from gsfpy.gsfRecords import ...' can be used with the version-specific implementations of the package it is necessary to mirror those implementations from the top level gsfpy namespace. This is now done in a generic way using dynamic import, so that future versions of GSF will also be supported without any code changes in the top-level gsfpy package being necessary.

eganjs · 2021-03-01T11:53:56Z

tests/gsfpy3_08/conftest.py

+# GSF_03_08_DATAFILE = GsfDatafile(
+#     GsfVersion.V03_08,
+#     Path(__file__).parent.parent
+#     / "test_data"
+#     / "0059_20181102_212138_EX1811_MB_EM302.gsf.mb121",
+#     num_beams=432,
+# )


commented data?

Fixed - the GSF 3.08 tests now use only GSF_03_08_DATAFILE test data

eganjs · 2021-03-01T11:54:12Z

tests/gsfpy3_08/conftest.py

+# @pytest.fixture
+# def gsf_test_data_03_08(tmp_path):
+#     yield from _setup_gsf_test_data(GSF_03_08_DATAFILE, tmp_path)


commented fixtures?

eganjs · 2021-03-01T11:54:25Z

tests/gsfpy3_08/conftest.py

+    [
+        gsf_test_data_03_06,
+        gsf_test_data_03_07,
+        # gsf_test_data_03_08,


should this be uncommented?

Done - GSF 3.08 tests now use only GSF 3.08 test data

eganjs · 2021-03-01T11:54:38Z

tests/gsfpy3_08/conftest.py

+    params=[
+        GSF_03_06_DATAFILE,
+        GSF_03_07_DATAFILE,
+        # GSF_03_08_DATAFILE,


should this be uncommented?

andyphelps · 2021-03-01T13:07:46Z

Makefile

+	poetry run pytest --ignore-glob=tests/gsfpy3_08/* --ignore-glob=tests/gsfpy3_09/* --ignore-glob=tests/gsfpy/test_gsffile_with_*.py --verbose --capture=no
+	poetry run pytest --ignore-glob=tests/gsfpy3_08/* --ignore-glob=tests/gsfpy3_09/* --ignore-glob=tests/gsfpy/test_gsffile.py --verbose --capture=no
+	poetry run pytest tests/gsfpy3_08/test_libgsf_load_valid.py --verbose --capture=no
+	poetry run pytest tests/gsfpy3_08/test_libgsf_load_invalid.py --verbose --capture=no
+	poetry run pytest tests/gsfpy3_08/test_libgsf_load_default.py --verbose --capture=no
+	poetry run pytest --ignore-glob=tests/gsfpy3_08/test_libgsf_load_*.py --ignore-glob=tests/gsfpy3_09/* --verbose --capture=no --cov=gsfpy3_08 --cov-fail-under=95 --cov-config=tox.ini
+	poetry run pytest tests/gsfpy3_09/test_libgsf_load_valid.py --verbose --capture=no
+	poetry run pytest tests/gsfpy3_09/test_libgsf_load_invalid.py --verbose --capture=no
+	poetry run pytest tests/gsfpy3_09/test_libgsf_load_default.py --verbose --capture=no
+	poetry run pytest --ignore-glob=tests/gsfpy3_09/test_libgsf_load_*.py --ignore-glob=tests/gsfpy3_08/* --verbose --capture=no --cov=gsfpy3_09 --cov-fail-under=95 --cov-config=tox.ini


Is there a reason these are run individually rather than just pointing pytest at the tests folder?

There is a reason! Once a module is loaded in python it is almost impossible to unload it. Therefore for tests which are checking the gsfpy behaviour of loading the correct version-specific namespace based on the DEFAULT_GSF_VERSION env variable, or loading custom versions of the libgsf shared object libraries, it is necessary to execute these in their own dedicated pytest runs.

andyphelps · 2021-03-01T13:10:02Z

README.md

+pip install git+ssh://[email protected]/UKHO/gsfpy3_08.git@master
 ```
 #### From GitHub (HTTPS)
 ```shell script
-pip install git+https://github.com/UKHO/gsfpy.git@master
+pip install git+https://github.com/UKHO/gsfpy3_08.git@master


Are these path's correct?

andyphelps · 2021-03-01T13:11:35Z

README.md

@@ -174,7 +181,7 @@ More recent versions of these documents can be downloaded from the
 By default Poetry will create it's own virtual environment using your system's Python. [This feature can be disabled.](https://python-poetry.org/docs/faq/#i-dont-want-poetry-to-manage-my-virtual-environments-can-i-disable-it)

 ```shell script
-git clone [email protected]:UKHO/gsfpy.git
+git clone [email protected]:UKHO/gsfpy3_08.git


andyphelps · 2021-03-01T13:11:43Z

README.md

@@ -184,7 +191,7 @@ poetry install
 A good choice if you want to run a version of Python different than available through your system's package manager

 ```shell script
-git clone [email protected]:UKHO/gsfpy.git
+git clone [email protected]:UKHO/gsfpy3_08.git


andyphelps · 2021-03-01T13:11:53Z

README.md

@@ -216,7 +223,7 @@ calling gsfpy to assess these risks and mitigate where deemed necessary.
 GSF data processed using gsfpy should be sourced from reliable providers
 and checked for integrity where possible.

-Please also refer to the LICENSE file for the terms of use of gsfpy.
+Please also refer to the LICENSE file for the terms of use of gsfpy3_08.


andyphelps · 2021-03-01T13:20:13Z

gsfpy/__init__.py

-        count = gsfGetNumberRecords(self._handle, desired_record)
-        _handle_failure(count)
-        return count
+def mirror_default_gsf_version_submodule(_globals: dict, submodule_name: str = None):


Perhaps rename the _globals to globals_dict. Leading underscore is misleading as it used for privacy etc. In this case I believe the _ is used to avoid clashing with the globals command?

Done, yes the original name was used to avoid the clash with globals(). Agree that globals_dict is a better alternative.

lmarsden added 11 commits February 23, 2021 18:26

Include embedded shared object libraries

293305e

Fix typo in CONTRIBUTING.md

21368c0

Additional test for GSF version set via environment

7c6cd5c

Add test coverage reporting

e83c933

* Note that the top-level gsfpy namespace is not included in coverage reporting as it simply acts as a switch between gsfpy3_08 and gsfpy3_09.

Additional tests

4f387aa

Update code examples in README

8490512

Set version to 2.0.0

0e82cd0

Reflect module structure of subpackages in top level namespace

6a7017c

* For improved backward compatibility it must be possible to do imports of the form 'from gsfpy.enums import ...'

Remove unneeded global

e1fd076

lmarsden marked this pull request as ready for review February 26, 2021 13:34

lmarsden requested a review from eganjs February 26, 2021 13:34

lmarsden linked an issue Feb 26, 2021 that may be closed by this pull request

Add support for GSF 3.09 #98

Closed

eganjs suggested changes Mar 1, 2021

View reviewed changes

andyphelps requested changes Mar 1, 2021

View reviewed changes

lmarsden added 3 commits March 1, 2021 13:34

Fix paths in README

72724bc

Fix comment in README

8e1574a

Rename parameter for improved clarity

5b95bc2

Base automatically changed from master to main March 1, 2021 13:52

Tidy up test data and associated documentation

3dfeec3

lmarsden requested review from eganjs and andyphelps March 1, 2021 14:51

eganjs previously approved these changes Mar 2, 2021

View reviewed changes

andyphelps previously approved these changes Mar 3, 2021

View reviewed changes

Add comment to Makefile to explain multiple pytest runs

38e96ab

lmarsden dismissed stale reviews from andyphelps and eganjs via 38e96ab March 3, 2021 08:58

lmarsden requested a review from andyphelps March 3, 2021 08:59

andyphelps approved these changes Mar 3, 2021

View reviewed changes

lmarsden merged commit 5ec4996 into main Mar 3, 2021

lmarsden deleted the support_gsf_3_09 branch August 16, 2021 11:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for GSF v3.09 #125

Add support for GSF v3.09 #125

lmarsden commented Feb 23, 2021 •

edited

Loading

eganjs Mar 1, 2021

lmarsden Mar 1, 2021

eganjs Mar 1, 2021

lmarsden Mar 1, 2021

eganjs Mar 1, 2021

lmarsden Mar 1, 2021

eganjs Mar 1, 2021

lmarsden Mar 1, 2021

andyphelps Mar 1, 2021

lmarsden Mar 1, 2021

andyphelps Mar 1, 2021

lmarsden Mar 1, 2021

andyphelps Mar 1, 2021

lmarsden Mar 1, 2021

andyphelps Mar 1, 2021

lmarsden Mar 1, 2021

andyphelps Mar 1, 2021

lmarsden Mar 1, 2021

andyphelps Mar 1, 2021

lmarsden Mar 1, 2021

Add support for GSF v3.09 #125

Add support for GSF v3.09 #125

Conversation

lmarsden commented Feb 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmarsden commented Feb 23, 2021 •

edited

Loading