SWOB to GeoJSON feedback

[thineshsornalingam]

@rjwest good work here https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojson.py

Some feedback:

• I like the data structure you're using internally to capture the content

• I don't see the qualifiers parse. Where are you storing the 'qa_summary', 'data_flag' etc values? Perhaps you can store the qualifiers as sub item in each element's list as an element itself since qualifies also have the same XML attributes as their parent elements.

• How are you handling 'code' uom? I don't see them in the structure for all the element, only some.

• Make sure to do proper logging, remove all print statements

• Make sure to run your code through pep8: https://www.python.org/dev/peps/pep-0008/. You can install pep8 like so https://pypi.org/project/pep8/ and run it against your code to clear any PEP8 style violations.

• Provide a test suite https://docs.python.org/3/library/unittest.html. Here's an example: https://github.com/woudc/woudc-extcsv/tree/master/tests

[thineshsornalingam]

Re: https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojsonV2.py

• https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojsonV2.py#L40 -- put method comments inside method

• https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojsonV2.py#L62 - instead of doing splits based on the os, you could use os.path.basename() to get the filename. The os lib will take care of Linux, Windows, etc.

• https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojsonV2.py#L71 -- instead of keeping error states, you could return here logging a CRITICAL message in the log that the provided SWOB could not parsed and thus method cannot continue further.

• https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojsonV2.py#L97 -- can we make this section generic? It could be that different SWOBs will have different identification elements, since we're hard coding the names we'll have to upkeep this section. Can we make it so that it reads all the elements in the identification section and saves their name, uom (code source, type), value in a generic manner given any SWOB?

• https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojsonV2.py#L158 -- for multiconditional boolean use python any or all syntax: https://www.geeksforgeeks.org/any-all-in-python/

• https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojsonV2.py#L161 -- good practice to use Python string formatting using '%s' instead of '+'

• https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojsonV2.py#L179 -- make swob2geojson the primary method that takes in the SWOB file path and calls parse_swob(filepath), this way the caller is not invoking two methods

[thineshsornalingam]

• Remove https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojson.py

• Create folder called 'test'. Rename 'test_files' to 'swob', rename 'test_master_files' to 'geojson'. Move both folder inside 'test'

• Remove 'examples'

[thineshsornalingam]

• Please fix style issues below:

(py3) sornalingamt@woudc-px-dev1:~/swob/SWOB-GeoJSON-master/src$ pycodestyle swob2geojsonV2.py
swob2geojsonV2.py:62:5: E722 do not use bare 'except'
swob2geojsonV2.py:142:21: E722 do not use bare 'except'
swob2geojsonV2.py:144:1: W293 blank line contains whitespace
swob2geojsonV2.py:201:15: W292 no newline at end of file

(py3) sornalingamt@woudc-px-dev1:~/swob/SWOB-GeoJSON-master/src$ pycodestyle run_tests.py
run_tests.py:40:67: W291 trailing whitespace
run_tests.py:107:1: E305 expected 2 blank lines after class or function definition, found 1

[thineshsornalingam]

• https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/swob2geojsonV2.py#L63 -- since this is a critical error, you can log it as LOGGER.critical

[thineshsornalingam]

Looks good @rjwest.

• Rename swob2geojsonV2.py to swob2geojson.py

• Move https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/src/run_tests.py into https://github.com/ECCC-PMCU/SWOB-GeoJSON/tree/master/test

[thineshsornalingam]

• Change string coordinates to float

[thineshsornalingam]

• https://github.com/ECCC-PMCU/SWOB-GeoJSON/blob/master/test/geojson/CAAW_minute_master.json#L13 -- change 'swob' to 'id'

[thineshsornalingam]

• [ ]

(py3) sornalingamt@woudc-px-dev1:~/swob/SWOB-GeoJSON-master/test$ python run_tests.py
Traceback (most recent call last):
File "run_tests.py", line 33, in
import swob2geojson as swob2gjson
ImportError: No module named 'swob2geojson'