Closed jhkennedy closed 5 years ago
Oh and I should mention, I leveraged PyCharm heavily for this refactor, so all the imports were "optimized" -- unused imports dropped, one package-per-line, absolute imports, alphabetized, etc.
This is great! I just pushed what I hope will be the last substantial changes to get everything working again. Looking over your changes everything is straightforward and makes sense. I would say go ahead and do the rebase/merge. I'll use this for the testing on cori/blues/acme1 over the next couple days and make any tweaks if errors come up.
Sounds good; will do!
re: testing. I'd like to setup some minimal test dataset on each of the machines so the tests can work anywhere, but its going to take a bit of work. Ideally that data could also then be used by new users for hello world examples.
@sterlingbaldwin rebase is done! Give it a whirl and go ahead and merge it if you think it's ready.
This:
Drops the
e3sm_
prefix from the package nameMoves all the processflow code down into a
processflow
package, which isolates the processflow code into it's own namespace (ine3sm_unified
currently, bothe3sm_processflow
ande3sm_to_cmip
create alib
package that shadowed each other).Moves the resource directory into the mane package and removes the
data_files
keyword insetup.py
, which is not recommended as it has many caveats. For example, see: https://stackoverflow.com/questions/47403188 . This prevented the resources directory from being discoverable when using a pip editable/setup.py develop install because it is left in repository (to be editable). By switching to an install that uses aMANIFEST.in
file, and settinginclude_package_data=True
insetup.py
, the resource directory will then be located relative to the processflow package path and available in an editable/develop install. The location of the resource directory can be found by:python -c "import os; from processflow import resources; print(os.path.dirname(resources.__file__))"
processflow
cli has been turned into an entrypoint, and now can be run viapython -m processflow [ARGS]
or using the createdprocessflow
script with a editable install:pip install -e . # once!; processflow [ARGS]
places processflow into testing mode is now done by passing the hidden
--test
argument to processflow, and the arguments can be passed toprocessflow.__main__.main
progromatically:main(['--test, '-h']) instead of needing to:
main(test=True, testargs=['-h']). That is, if the--test
argument is present, processflow assumes all other arguments are the testargs.some style changes were made to better conform to PEP8 where helpful for this refactor, but overall style was ignored.
Right now, the same tests pass from before the branch off of
nightly
( dd99b42 ) to this refactor. However, since a lot of the tests make assumptions (users permissions, paths, and machine), I'm not able to run all the tests.@sterlingbaldwin , due to the significance of the changes here, the best path to merging would likely be to rebase these changes on top of the current HEAD of nightly.
I can do the rebase and force-push the changes to this branch (updating this PR) once your:
And then we can see how the nightly test go.