Skip to content

Refactor epsgRef to use JSON file in platform independent location#422

Merged
jdhughes-dev merged 1 commit into
modflowpy:developfrom
mwtoews:epsg-ref
Oct 31, 2018
Merged

Refactor epsgRef to use JSON file in platform independent location#422
jdhughes-dev merged 1 commit into
modflowpy:developfrom
mwtoews:epsg-ref

Conversation

@mwtoews
Copy link
Copy Markdown
Contributor

@mwtoews mwtoews commented Oct 22, 2018

The previous implementation stored user data within a Python module in a platform specific location, which is problematic on many levels.

For instance, on platforms that don't use site-packges, this is the error:

   File "../flopy/utils/reference.py", line 1812, in __init__
    sp = [f for f in sys.path if f.endswith('site-packages')][0]
IndexError: list index out of range

Furthermore, often this directory is not user-writable, therefore many of the tricks to read/write epsgref.py would not work without root privileges. And there are too many workarounds required to make a Python module behave as hashable and human-readable database (e.g. reload(), deleting .pyc, etc.).

I think it's better to store the user data using appdirs, which effortlessly finds the platform independent and writable user data directory. Furthermore, use a JSON to store the EPSG coordinate reference system data. This PR uses an OrderedDict to maintain the insertion order, in case it helps to inspect any issues.

While re-doing parts of the Notebook, I noticed that "junk" gets stored with this PR, but it seems this should have been the case with the previous implementation too.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.008%) to 69.322% when pulling 1db48a5 on mwtoews:epsg-ref into 5cf3d83 on modflowpy:develop.

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 23, 2018

Coverage Status

Coverage decreased (-0.0002%) to 69.319% when pulling 22b8b55 on mwtoews:epsg-ref into e71fbbd on modflowpy:develop.

@jdhughes-dev
Copy link
Copy Markdown
Contributor

@mwtoews Since appdirs is not a standard python package can you wrap the import in a try and except. If it fails an import exception should probably be issued. You should also add this dependency to the optional method dependencies.

Feel free to improve the docstrings on the epsgRef!

Replace previous logic for epsgref.py as a database with a similar
JSON structure. New storage location is platform independent, located
using appdirs package (if available), otherwise in 'HOME/.flopy'
@mwtoews
Copy link
Copy Markdown
Contributor Author

mwtoews commented Oct 30, 2018

Thanks @jdhughes-usgs , those are reasonable suggestions. I've set the fall-back location in 'HOME/.flopy' if appdirs is not available. (Btw, I see 'appdirs' bundled in with Anaconda, but not with Miniconda).

I was considering adding 'appdirs' as a dependency to the optional methods dependencies section, however it's not so simple to map this dependency out to particular functions. It's also not a vital dependency either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants