openmm / pdbfixer

PDBFixer fixes problems in PDB files
Other
453 stars 114 forks source link

GitHub Action to lint Python code #222

Closed cclauss closed 2 years ago

cclauss commented 3 years ago

Travis CI seems to be on (permanent?) vacation.

Results: https://github.com/cclauss/pdbfixer/actions

peastman commented 3 years ago

Thanks! Getting CI working again would be a very good thing.

One of the builds is failing at the installation stage:

ERROR: Could not find a version that satisfies the requirement openmm>=7.1 (from pdbfixer) (from versions: 0.1, 0.2) ERROR: No matching distribution found for openmm>=7.1

You're running a bunch of checks that I don't think make sense. For example, codespell reports lots of "errors" in things it just doesn't know, like thinking "SER" is a mispelling of "SET". And flake8 reports lots of "errors" for lines being longer than 88 characters, when in fact when don't impose any hard limit on line length.

I would keep it to just checking for actual errors: make sure everything installs, and all the test cases pass.

cclauss commented 3 years ago
  1. pip install openmm fails because it has not been updated since early 2014`
  2. Warning: importing 'simtk.openmm' is deprecated. Import 'openmm' instead.
jchodera commented 3 years ago

Thanks so much for trying to get this working, @cclauss!

@peastman : What would you think of using the MolSSI Cookiecutter workflow? We've been trying to standardize the computational chemistry community around the cookiecutter-generated GitHub Actions and repository structure to make it easier for others to contribute to other packages, and encode best practices for organizing documentation, automating unit tests, and the like.

cclauss commented 3 years ago

This PR is dropping back to linting Python code with bandit, codespell, and flake8.

peastman commented 3 years ago

What would you think of using the MolSSI Cookiecutter workflow?

I don't understand their structure. Inside .github there are two folders, one called reference-workflows and one just called workflows. The former contains a whole lot of yaml files with cryptic names, and while there's a readme, I found it mostly unenlightening. The latter contains no readme and only a single file called verify-ghas.yaml.

Looking at the top level readme, I get the impression (but it's not entirely clear) that the github actions files aren't actually designed for testing your project. They're to test the cookie cutter itself.

jchodera commented 3 years ago

I don't understand their structure.

That's because you're looking at the CI for the cookiecutter template itself. Try running the cookiecutter or looking at the templated CI.yaml that would get installed.

peastman commented 3 years ago

Are you suggesting that we adapt that yaml file for CI, or that we regenerate the whole repository using the cookie cutter?

jchodera commented 3 years ago

We can instantiate the cookiecutter in an empty repository and migrate the components we want to use, such as the GitHub Actions workflow and even the sphinx docs template.

peastman commented 3 years ago

Ok. Using the GHA workflow from the cookiecutter sounds reasonable to me.

cclauss commented 3 years ago

The thing that I could not figure out in this PR was how to use Conda to install the current code in this repo. I know how to pip install -e . but do not know how to do the same with Conda.

swails commented 2 years ago

The thing that I could not figure out in this PR was how to use Conda to install the current code in this repo. I know how to pip install -e . but do not know how to do the same with Conda.

Don't do the same with conda. Load a conda environment with pip, then pip install -e . :shrug:

peastman commented 2 years ago

This is superseded by #225.

cclauss commented 2 years ago

How does #225 supersede this PR? It runs none of these tests and it fixes none of the issues.

peastman commented 2 years ago

As I said above, I think a lot of these checks aren't really appropriate. You're checking for compliance with a coding standard we don't follow. My advice was, "Make sure everything installs, and all the test cases pass." That's what #225 does.