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

feat(get_modflow): support modflow6 repo releases #1573

Merged
merged 2 commits into from Oct 11, 2022

Conversation

wpbonelli
Copy link
Contributor

@wpbonelli wpbonelli commented Oct 6, 2022

Supports installing releases from the main modflow6 repository. Updates the repo option to accept modflow6 so the user can select any of the following:

  • executables (default)
  • modflow6
  • modflow6-nightly-build

The modflow6 repo's release archive contains most of the repository, not just binaries, so instead of unzipping directly to bindir like the other 2 options, there is an extra step to unzip in the download location before copying binaries to bindir.

Adds a section describing repo selection to the docs. Also adds a table of contents.

Motivation

Besides covering the bases as far as distributions go, this is a step towards replacing the pymake dependency in modflow6 tests (currently pymake is used to download the latest mf6 release, among other things)

@wpbonelli wpbonelli requested a review from mwtoews October 6, 2022 05:06
@wpbonelli wpbonelli force-pushed the update-get-modflow branch 2 times, most recently from f758dc3 to 4a441ad Compare October 6, 2022 05:14
@codecov
Copy link

codecov bot commented Oct 6, 2022

Codecov Report

Merging #1573 (ef590eb) into develop (a0ca344) will increase coverage by 0.1%.
The diff coverage is 95.2%.

@@            Coverage Diff            @@
##           develop   #1573     +/-   ##
=========================================
+ Coverage     72.5%   72.6%   +0.1%     
=========================================
  Files          251     251             
  Lines        54293   54423    +130     
=========================================
+ Hits         39377   39542    +165     
+ Misses       14916   14881     -35     
Impacted Files Coverage Δ
flopy/utils/get_modflow.py 54.7% <95.2%> (+4.8%) ⬆️
flopy/mf6/data/mfdataarray.py 62.3% <0.0%> (-1.2%) ⬇️
flopy/utils/datautil.py 63.4% <0.0%> (-0.5%) ⬇️
flopy/mf6/data/mfdatalist.py 72.9% <0.0%> (-0.2%) ⬇️
flopy/mf6/mfpackage.py 64.3% <0.0%> (+<0.1%) ⬆️
flopy/mf6/data/mfdatastorage.py 70.6% <0.0%> (+1.2%) ⬆️
flopy/plot/map.py 83.9% <0.0%> (+1.5%) ⬆️
flopy/mf6/data/mfdatautil.py 54.3% <0.0%> (+1.6%) ⬆️
flopy/mf6/data/mffileaccess.py 66.0% <0.0%> (+2.1%) ⬆️
... and 1 more

@wpbonelli wpbonelli force-pushed the update-get-modflow branch 2 times, most recently from 6d52ae8 to e4cf9b9 Compare October 6, 2022 14:22
flopy/utils/get_modflow.py Outdated Show resolved Hide resolved
@wpbonelli wpbonelli force-pushed the update-get-modflow branch 2 times, most recently from b647fa0 to d045f41 Compare October 6, 2022 21:33
@wpbonelli wpbonelli marked this pull request as ready for review October 6, 2022 22:13
@jdhughes-usgs
Copy link
Contributor

Or it can append new or modify existing entries in code.json. We probably need to harmonize the attributes/keys in the code.json files.

@wpbonelli wpbonelli marked this pull request as draft October 10, 2022 16:50
@wpbonelli wpbonelli marked this pull request as ready for review October 10, 2022 18:38
if ostag in asset["name"]:
break

# Windows asset in modflow6 repo release has no OS tag
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this can be removed later if future modflow6 releases use the same naming conventions for platform-specific zip files as the executables and nightly-build repos

  • mac.zip
  • linux.zip
  • win64.zip

* avoid div/0 with columns_str (with items longer than 79 chars)
* change asset dst_fname for modflow6
* prevent "--repo modflow6 --ostag win32" from succeding
Copy link
Contributor

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

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

I've pushed a commit for a few changes. Roll back or comment anything that you disagree with. Here are my notes from my commit message:

  • evaluate files to extract (rather than extracting to dowloads_dir)
  • avoid div/0 with columns_str (with items longer than 79 chars)
  • change asset dst_fname for modflow6
  • prevent "--repo modflow6 --ostag win32" from succeding

@wpbonelli
Copy link
Contributor Author

wpbonelli commented Oct 11, 2022

@mwtoews thanks for the review and fixes

My motivation for extracting the whole release instead of select files, besides not being sure which would be a less surprising option for the user, was because modflow6 tests need to download the latest release and rebuild it — currently this is done with pymake.download_and_unzip but I thought to replace it with get-modflow (with the wider goal of moving testing/miscellaneous utils to modflow-devtools and paring down pymake)

If this seems like a separate capability that doesn't belong here, the modflow6 tests could just pull in download_and_unzip from modflow-devtools instead After some thought this seems best. Downloading the repo for dev/testing doesn't seem like a concern of flopy's

@langevin-usgs langevin-usgs merged commit b8d471c into modflowpy:develop Oct 11, 2022
@wpbonelli wpbonelli deleted the update-get-modflow branch October 11, 2022 17:56
wpbonelli added a commit to wpbonelli/flopy that referenced this pull request Dec 14, 2022
* feat(get_modflow): support modflow6 repo releases

* * evaluate files to extract (rather than extracting to dowloads_dir)
* avoid div/0 with columns_str (with items longer than 79 chars)
* change asset dst_fname for modflow6
* prevent "--repo modflow6 --ostag win32" from succeding

Co-authored-by: Mike Taves <mwtoews@gmail.com>
wpbonelli added a commit to wpbonelli/flopy that referenced this pull request Dec 14, 2022
* feat(get_modflow): support modflow6 repo releases

* * evaluate files to extract (rather than extracting to dowloads_dir)
* avoid div/0 with columns_str (with items longer than 79 chars)
* change asset dst_fname for modflow6
* prevent "--repo modflow6 --ostag win32" from succeding

Co-authored-by: Mike Taves <mwtoews@gmail.com>
wpbonelli added a commit that referenced this pull request Dec 14, 2022
* feat(get_modflow): support modflow6 repo releases

* * evaluate files to extract (rather than extracting to dowloads_dir)
* avoid div/0 with columns_str (with items longer than 79 chars)
* change asset dst_fname for modflow6
* prevent "--repo modflow6 --ostag win32" from succeding

Co-authored-by: Mike Taves <mwtoews@gmail.com>
wpbonelli added a commit to wpbonelli/flopy that referenced this pull request Dec 14, 2022
* feat(get_modflow): support modflow6 repo releases

* * evaluate files to extract (rather than extracting to dowloads_dir)
* avoid div/0 with columns_str (with items longer than 79 chars)
* change asset dst_fname for modflow6
* prevent "--repo modflow6 --ostag win32" from succeding

Co-authored-by: Mike Taves <mwtoews@gmail.com>
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