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

refactor(get-modflow): add option to have flopy bindir on PATH #1511

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

mwtoews
Copy link
Contributor

@mwtoews mwtoews commented Aug 24, 2022

The main ambition in this PR is to allow a user to easily install executables for flopy to use. The steps would be:

  1. pip install flopy / conda install flopy
  2. get-modflow :flopy

The auto-select option :flopy for bindir will determine the path to the users' localdata directory, which will use a bin directory. This localdata bin directory will be prepended to Python's PATH after import flopy so that most m.run_model() commands should find a working executable. There are a few downsides or things to consider when choosing :flopy as a recommended default:

  • The localdata bin directory used by flopy is only added to PATH within Python after import flopy, but not before the import and not in a command prompt (unless that localdata bin directory is manually added to PATH)
  • The approach taken by import flopy is to always prepend the localdata bin directory, even if it doesn't exist. (This allows a user to type import flopy; flopy.utils.get_modflow(":flopy") in a Python prompt to install a new localdata bin setup with a fresh install without needing to restart Python).
  • If a user has a different (e.g.) mf2005 on their PATH, then potentially two different versions might be used, i.e. one for flopy and one for their command prompt. This could get confusing for a user.
  • Users that expect to see the same (e.g.) mf2005 executable used for console and in flopy should not choose :flopy, but a "higher-up" directory in a user's PATH.

Other changes in this PR:

  • Add simple install instructions to documentation
  • Write metadata history for get-modflow -- this gets stored in the same localdata directory for flopy, and stores a few bits of info, such as the md5sum check from code.json (was this needed?) and selections for each bindir
  • Change bindir ? option to : -- this change is because bash uses ? to match one-character paths, so a different meta-option was required.
  • Add other bindir auto-select options, including (e.g.) :flopy as discussed above.
  • If get-modflow encounters "HTTP Error 503: Egress is over the account limit" (reported here), retry (up to 3 times).
  • Rename flopy.utils.get_modflow_main to flopy.utils.get_modflow and flopy.utils.get_modflow to flopy.utils.get_modflow_module
  • Rename test_scripts.py to test_get_modflow.py

@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #1511 (39fb9b5) into develop (c386c66) will decrease coverage by 0.0%.
The diff coverage is 31.0%.

@@            Coverage Diff            @@
##           develop   #1511     +/-   ##
=========================================
- Coverage     72.4%   72.4%   -0.1%     
=========================================
  Files          251     251             
  Lines        54053   54166    +113     
=========================================
+ Hits         39184   39228     +44     
- Misses       14869   14938     +69     
Impacted Files Coverage Δ
flopy/utils/get_modflow.py 50.1% <27.8%> (-6.1%) ⬇️
flopy/mbase.py 68.3% <100.0%> (+0.1%) ⬆️
flopy/utils/__init__.py 100.0% <100.0%> (ø)

@mwtoews mwtoews force-pushed the ref-get-modflow-meta branch 2 times, most recently from f844f84 to b04eb7a Compare August 25, 2022 03:00
@mwtoews
Copy link
Contributor Author

mwtoews commented Aug 25, 2022

I've popped over to a Windows PC to see how this works, and it seems to be fine. Here is a Python demo:

import flopy

flopy.utils.get_modflow(":flopy")
# fetched release '8.0' from MODFLOW-USGS/executables ...
# extracting 25 files to 'C:\Users\mtoews\AppData\Local\flopy\bin' ...
# wrote new flopy metadata file: 'C:\Users\mtoews\AppData\Local\flopy\get_modflow.json'

# build or load mf6 model
sim.run_simulation()
# FloPy is using the following executable to run the model: C:\Users\mtoews\AppData\Local\flopy\bin\mf6.exe
#                                    MODFLOW 6
#                 U.S. GEOLOGICAL SURVEY MODULAR HYDROLOGIC MODEL
#                             VERSION 6.3.0 03/04/2022
# ...

# overwrite some executables with nightly build
flopy.utils.get_modflow(":flopy", repo="modflow6-nightly-build")
# fetched release '20220824' from MODFLOW-USGS/modflow6-nightly-build ...
# extracting 4 files to 'C:\Users\mtoews\AppData\Local\flopy\bin'
# libmf6.dll mf5to6.exe mf6.exe    zbud6.exe
# updated flopy metadata file: 'C:\Users\mtoews\AppData\Local\flopy\get_modflow.json'
sim.run_simulation()
# FloPy is using the following executable to run the model: C:\Users\mtoews\AppData\Local\flopy\bin\mf6.exe
#                                    MODFLOW 6
#                 U.S. GEOLOGICAL SURVEY MODULAR HYDROLOGIC MODEL
#                    VERSION 6.4.0 release candidate 08/24/2022
#                                ***DEVELOP MODE***
# 
#    MODFLOW 6 compiled Aug 24 2022 04:07:04 with Intel(R) Fortran Intel(R) 64
# ...

# restore release versions of mf6 and libmf6
flopy.utils.get_modflow(":flopy", subset={"mf6", "libmf6"})
# fetched release '8.0' from MODFLOW-USGS/executables ...
# extracting 2 files to 'C:\Users\mtoews\AppData\Local\flopy\bin'
# libmf6.dll (6.3.0) mf6.exe (6.3.0)
# updated flopy metadata file: 'C:\Users\mtoews\AppData\Local\flopy\get_modflow.json'

the metadata captured in C:\Users\mtoews\AppData\Local\flopy\get_modflow.json is:

[
    {
        "bindir": "C:\\Users\\mtoews\\AppData\\Local\\flopy\\bin",
        "owner": "MODFLOW-USGS",
        "repo": "executables",
        "release_id": "8.0",
        "name": "win64.zip",
        "updated_at": "2022-03-08T01:37:45Z",
        "extracted_at": "2022-08-25T14:35:03.140890",
        "code_json_md5": "21f6cad36dcf0796342c94835a05724a"
    },
    {
        "bindir": "C:\\Users\\mtoews\\AppData\\Local\\flopy\\bin",
        "owner": "MODFLOW-USGS",
        "repo": "modflow6-nightly-build",
        "release_id": "20220824",
        "name": "win64.zip",
        "updated_at": "2022-08-24T04:11:16Z",
        "extracted_at": "2022-08-25T14:37:26.766378"
    },
    {
        "bindir": "C:\\Users\\mtoews\\AppData\\Local\\flopy\\bin",
        "owner": "MODFLOW-USGS",
        "repo": "executables",
        "release_id": "8.0",
        "name": "win64.zip",
        "updated_at": "2022-03-08T01:37:45Z",
        "extracted_at": "2022-08-25T14:45:05.610896",
        "subset": [
            "libmf6",
            "mf6"
        ],
        "code_json_md5": "21f6cad36dcf0796342c94835a05724a"
    }
]

Copy link
Contributor

@langevin-usgs langevin-usgs left a comment

Choose a reason for hiding this comment

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

This looks really nice, @mwtoews. Great contribution.

docs/get_modflow.md Outdated Show resolved Hide resolved
Copy link
Contributor

@wpbonelli wpbonelli left a comment

Choose a reason for hiding this comment

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

Storing metadata is nice. The code.json hash might allow feature toggling, if there's ever a use case for that. I wonder if it might be useful to store the entire contents of code.json for each metadata entry, so flopy can inspect specific executable versions, but the same info could be retrieved via github API if needed.

Is there a case against auto-selecting the :flopy install location as default when no argument is provided? So in a clean environment one could just pip / conda install flopy then get-modflow. Prompts to help select a location could show if executables are detected on the path maybe? Displaying the guidance you have above or something similar

Users that expect to see the same (e.g.) mf2005 executable used for console and in flopy should not choose :flopy, but a "higher-up" directory in a user's PATH.

Very minor but maybe also to make it a little more discoverable from python, naming get_modflow_main to get_modflow to follow the command line usage?

@mwtoews mwtoews force-pushed the ref-get-modflow-meta branch 2 times, most recently from 0085ab2 to f6eb91f Compare August 26, 2022 11:31
@mwtoews
Copy link
Contributor Author

mwtoews commented Aug 26, 2022

Storing metadata is nice. The code.json hash might allow feature toggling, if there's ever a use case for that. I wonder if it might be useful to store the entire contents of code.json for each metadata entry, so flopy can inspect specific executable versions, but the same info could be retrieved via github API if needed.

I considered repeating more from code.json, but that seemed too bulky and unnecessary. The info captured is the minimum necessary to understand the sequence of what/where was extracted.

Is there a case against auto-selecting the :flopy install location as default when no argument is provided? So in a clean environment one could just pip / conda install flopy then get-modflow. Prompts to help select a location could show if executables are detected on the path maybe? Displaying the guidance you have above or something similar

I prefer to have bindir to be optional, given that it may not suite everyone's needs. And this option does not work if the script is taken out of flopy and run stand-alone. However, it should be documented a bit more. I've added some "Examples" to the bottom when you type get-modflow --help.

Very minor but maybe also to make it a little more discoverable from python, naming get_modflow_main to get_modflow to follow the command line usage?

Good idea, and done! To get around the name conflict (that the filename is the same as the function), the module is renamed to flopy.utils.get_modflow_module. If desired, the documentation could also show flopy.utils.get_modflow(":flopy") too. I've updated the example above to reflect the current status. Thanks for the feedback!

@wpbonelli
Copy link
Contributor

And this option does not work if the script is taken out of flopy and run stand-alone.

Didn't think of that- in retrospect I think you mentioned this consideration in the call some weeks ago. Thanks!

@langevin-usgs
Copy link
Contributor

Hey @mwtoews, looks like there is a merge conflict. Can you resubmit and we'll merge it in?

- With "import flopy" append a local flopy bindir to the the PATH
- Add simple install instructions to documentation
- Write metadata history for get-modflow
- Change bindir "?" option to ":"
- Add other bindir auto-select options ":flopy", ":python", etc.
- Add examples at the bottom with --help option
- If get-modflow encounters HTTP error 503, retry (up to 3 times)
- Rename "flopy.utils.get_modflow_main" to "flopy.utils.get_modflow"
  and "flopy.utils.get_modflow" to "flopy.utils.get_modflow_module"
- Rename "test_scripts.py" to "test_get_modflow.py"
@mwtoews
Copy link
Contributor Author

mwtoews commented Aug 28, 2022

Last edits are done, and it's ready to merge.

Final edits (sorry, changes were wiped with push -f) were to change the numbered :1, :2, etc. auto-select options with named :prev, :python options, which have documentation. Also, get_modflow.json is not updated if detected to be run with pytest.

@langevin-usgs langevin-usgs merged commit 8f93d12 into modflowpy:develop Aug 29, 2022
@mwtoews mwtoews deleted the ref-get-modflow-meta branch August 30, 2022 00:27
wpbonelli pushed a commit to wpbonelli/flopy that referenced this pull request Dec 14, 2022
…owpy#1511)

- With "import flopy" append a local flopy bindir to the the PATH
- Add simple install instructions to documentation
- Write metadata history for get-modflow
- Change bindir "?" option to ":"
- Add other bindir auto-select options ":flopy", ":python", etc.
- Add examples at the bottom with --help option
- If get-modflow encounters HTTP error 503, retry (up to 3 times)
- Rename "flopy.utils.get_modflow_main" to "flopy.utils.get_modflow"
  and "flopy.utils.get_modflow" to "flopy.utils.get_modflow_module"
- Rename "test_scripts.py" to "test_get_modflow.py"
wpbonelli pushed a commit to wpbonelli/flopy that referenced this pull request Dec 14, 2022
…owpy#1511)

- With "import flopy" append a local flopy bindir to the the PATH
- Add simple install instructions to documentation
- Write metadata history for get-modflow
- Change bindir "?" option to ":"
- Add other bindir auto-select options ":flopy", ":python", etc.
- Add examples at the bottom with --help option
- If get-modflow encounters HTTP error 503, retry (up to 3 times)
- Rename "flopy.utils.get_modflow_main" to "flopy.utils.get_modflow"
  and "flopy.utils.get_modflow" to "flopy.utils.get_modflow_module"
- Rename "test_scripts.py" to "test_get_modflow.py"
wpbonelli pushed a commit that referenced this pull request Dec 14, 2022
- With "import flopy" append a local flopy bindir to the the PATH
- Add simple install instructions to documentation
- Write metadata history for get-modflow
- Change bindir "?" option to ":"
- Add other bindir auto-select options ":flopy", ":python", etc.
- Add examples at the bottom with --help option
- If get-modflow encounters HTTP error 503, retry (up to 3 times)
- Rename "flopy.utils.get_modflow_main" to "flopy.utils.get_modflow"
  and "flopy.utils.get_modflow" to "flopy.utils.get_modflow_module"
- Rename "test_scripts.py" to "test_get_modflow.py"
wpbonelli pushed a commit to wpbonelli/flopy that referenced this pull request Dec 14, 2022
…owpy#1511)

- With "import flopy" append a local flopy bindir to the the PATH
- Add simple install instructions to documentation
- Write metadata history for get-modflow
- Change bindir "?" option to ":"
- Add other bindir auto-select options ":flopy", ":python", etc.
- Add examples at the bottom with --help option
- If get-modflow encounters HTTP error 503, retry (up to 3 times)
- Rename "flopy.utils.get_modflow_main" to "flopy.utils.get_modflow"
  and "flopy.utils.get_modflow" to "flopy.utils.get_modflow_module"
- Rename "test_scripts.py" to "test_get_modflow.py"
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.

None yet

4 participants