-
Notifications
You must be signed in to change notification settings - Fork 12
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
Package and move ngen cal into ngen
namespace package
#30
Conversation
@hellkite500, I can't add you as a reviewer, so pinging you instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks alright to me in general, but we should fix the failing package test in this action.
note, ngen_cal depends on ngen_conf. ngen_cal tests against ngen_conf code pulled by the runner, NOT what ngen_cal has specified in setup.cfg.
.github/workflows/ngen-conf.yml
Outdated
deactivate | ||
rm -rf venv | ||
|
||
- name: Install ngen_cal and run unit tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this be more suited to a name indicating the tests are actually being run?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that is fair. I broke apart the workflow into more stages and changed the stage naming convention to use present tense as suggested.
Unless there are other changes that you would like me to make, @hellkite500, I think this should be ready to merge. |
The original CI tests are failing because of the changes to the package structure. However, the updated CI workflows are passing. |
Moves
ngen_cal
underngen.cal
namespace and adds necessary metadata files (e.g.setup.cfg
,pyproject.toml
) to installngen.cal
usingpip
. This should make it easier for downstream users to install and get started.Changes
python -m ngen.cal
.ngen.cal
instead ofngen_cal
.python3 -m pip install "git+https://github.com/noaa-owp/ngen-cal@master#egg=ngen_cal&subdirectory=python/ngen_cal"
).ngen_cal
andngen_conf
tests separately. Note,ngen_cal
tests againstngen_conf
code pulled by the runner, not whatngen_cal
has specified insetup.cfg
.Testing
Notes
pip install -e ".[develop]"
. This will install the package in "edit" mode along with the packages used for testing (pytest
).Checklist
Testing checklist
Target Environment support