gmdsi / GMDSI_notebooks

python-based predictive groundwater modeling workflow examples
GNU General Public License v3.0
48 stars 31 forks source link

Mods from notebook run throughs #82

Closed briochh closed 10 months ago

briochh commented 10 months ago

Mostly minor, so far.

I have moved to use pip -e in the conda environment file to install the local (distributed in this repo) version of pyemu and flopy. This should avoid the need for the path wrangling at each tutorial run time. The flopy and pyemu version in the repo are current version downloaded from GitHub. We could move link pip straight to a git version of these repos so we could avoid maintaining here.

More to come

jtwhite79 commented 10 months ago

@briochh I pulled your develop branch locally, recreated the env but something is up with the assert that is checking where flopy and pyemu are located:

image

I guess this has to do with the pip -e stuff? Any ideas on this? If I skip the assert and continue, then I run into this:

image

this on the part2 setup pstfrom notebook. It looked like some of the flopy src files are rm'd in your commits but I may have misread that...

briochh commented 10 months ago

Yeah. Need to strip out sys path append dependencies.

jtwhite79 commented 10 months ago

So does this mean those asserts dont work with pip -e? looks like the __file__ attr is None...I like those asserts just because it keeps someone from creating the env, the updating flopy/pyemu, and then telling us that something isnt working! I may or may not have experienced exactly this issue!

briochh commented 10 months ago

Asserts should still work. In fact, I think they kinda are. The problem is that with “../../dependencies” added to the path, python will import the parent flopy and pyemu directories (which contain no source). If you strip out the sys.path manipulation then python should import from the environment location (which is in dependencies/pyemu/pyemu now) and file should still contain “dependencies”

jtwhite79 commented 10 months ago

doh! you are right. I see it now - working as expected if I comment out the sys path insert

briochh commented 10 months ago

We can always change it back. Was an attempt to simplify, maybe it doesn’t.

jtwhite79 commented 10 months ago

I'm good with the pip -e approach - I think the implied best practices are an important piece of these notebooks and it seems like pip -e with conda/mamba (and esp conda pack) is cleaner...

briochh commented 10 months ago

@jtwhite79, @rhugman, this should be g2g now. Nothing too major.