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

HARMONY-388: Better consistency across Python repos #5

Merged
merged 6 commits into from
Jun 8, 2021

Conversation

vinnyinverso
Copy link
Contributor

What follows are the acceptance criteria and what I found or did to address them. (I also spoke with Chris prior to submitting this PR.)

  • Consistent python environment setup and use across the 3 repos
    • For all but the harmony-service example, the READMEs were suggesting to use some combination of pyenv, pyenv-virtualenv, or venv. I tried to update the READMEs so that it was clear to the reader that all of these were acceptable tools.
  • Either build scripts or make across the 3 repos
    • All of the regular (non-Docker service) Python module/lib repos (harmony-service-lib, harmony-py) use make files. All of the Docker/service-oriented repos (harmony-netcdf-to-zarr, harmony-service-example) use build scripts because they seem more conducive to the scripting logic needed to satisfy some of the more complex build logic. This seemed like a reasonably consistent solution (makefiles for pure libraries and build scripts for docker services).
  • Consistent way of installing deps across the 3 repos
    • All except one repo are using pip for installing dependencies in a fairly consistent manner, with one requirements file for dev, and one for core requirements. The only repo not following this pattern is the harmony-service-example, which uses conda. However, conda is a recommended way of installing one of the main dependencies of this repo (gdal) whereas pip is not (and is difficult to use in this instance from my experience). The use of conda, is also still compatible with the aforementioned recommended Python version manager and virtual env tools.
  • Consistent linting across the 3 repos
    • All repos (except for harmony-service-example) use flake8. We could add flake8 to harmony-service-example if desirable. I'm guessing it wasn't added due to the simplicity of the example.
  • Consistent unit testing framework and execution across the 3 repos
    • I believe Patrick said there was another ticket addressing this.
  • Consistent CI/CD across the 3 repos
    • Much of this seems to be adequately addressed by the recent transition to GitHub and related work.
  • Update READMEs across the 3 repos to document the consistent tools, packages, and configuration for setting up a dev environment; dev environment does not presume editor
    • I updated 3 READMEs to give more consistent suggestions on virtual environment setup. None of the dev environment setups assume the user has a specific editor.

.gitignore Outdated
@@ -113,6 +113,7 @@ venv/
ENV/
env.bak/
venv.bak/
harmony-ntz
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious - what is this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The readme recommended creating a virtual environment called harmony-ntz with pyenv-virtualenv. If you use venv to do the same, it creates a directory with that name for the virtual environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm guessing people have not been using the virtual environment naming suggested in the readme in favor of something like "venv". (I used harmony-ntz when I created my virtual environment because that seemed to be what was being suggested.)

Copy link
Member

Choose a reason for hiding this comment

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

I've been using pyenv virtualenv, but for consistency with ourselves and other projects I've seen and easier setup of $PATH, it may be better to just have the environment called env (or venv or you pick one) in every project, so we can run the same source env/bin/activate everywhere and have the same gitignore everywhere.

Copy link
Member

@bilts bilts left a comment

Choose a reason for hiding this comment

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

Nice work. If you're editing the README again, you may want to mention up front that you can also just docker pull harmonyservices/netcdf-to-zarr

.gitignore Outdated
@@ -113,6 +113,7 @@ venv/
ENV/
env.bak/
venv.bak/
harmony-ntz
Copy link
Member

Choose a reason for hiding this comment

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

I've been using pyenv virtualenv, but for consistency with ourselves and other projects I've seen and easier setup of $PATH, it may be better to just have the environment called env (or venv or you pick one) in every project, so we can run the same source env/bin/activate everywhere and have the same gitignore everywhere.

@vinnyinverso vinnyinverso changed the title HARMONY-388: Update README guidance on dev environment setup HARMONY-388: Better consistency across Python repos May 28, 2021
@vinnyinverso vinnyinverso merged commit 90fdb97 into main Jun 8, 2021
@vinnyinverso vinnyinverso deleted the HARMONY-388 branch June 8, 2021 13:36
@vinnyinverso vinnyinverso restored the HARMONY-388 branch June 8, 2021 18:23
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.

3 participants