Update standards based on Nov 2019 PyHC meeting discussion #16
Conversation
This still needs authors/contributors of the revision added, and to be reviewed during one of our telecons.
We discussed the amendments from the fall 2019 meeting during a PyHC telecon on 06-Jan-2020, and approved the changes. We still need to update the list of authors of the amendment. One topic of discussion during this telecon was on adoption of NEP 29 (which created a community-wide standard for how long to support older versions of Python and SciPy) by PyHC packages. Stuart Mumford suggested that it be okay for some packages to have stricter standards, and that packages can support older versions of NumPy and Python if they so choose. Michael Hirsch brought up that usage/download statistics from a package that supports Python 3.5+ showed that very few people are using Python 3.5 at the moment (and also that Python 3.5 gives the most trouble for the testing matrix). I brought up that core packages like SunPy should follow the guidelines of NEP 29 most closely, while packages that are currently under development and have few users would be okay in supporting Python 3.7+ or even 3.8+ if the development phase is going to last ~1-2 years. We decided that using "should" leaves open enough leeway for us to accept packages that have different requirements. I forgot to mention that I think that we should follow the same guidelines for Astropy (and ideally packages within PyHC) as we do for NumPy. (These notes are a convolution of what people actually said, and my ability to remember 42 minutes into the past.)
|
If you approve these changes and would like to be listed as a co-author, please let me know! You can add a comment, suggest a commit, or approve this pull request. It'd be helpful to know the name/affiliation you would like used if they're different from the original document, as well as your ORCID if you have one. Many thanks to @ehsteve and everyone else who shepherded the process the first time around! |
|
We also have been talking about who is willing to volunteer for going through PyHC packages and evaluating the extent to which each package meets the standards. We could potentially do this during the hackathon following our spring meeting in Cambridge. The main meeting will be from April 27-29, with the hackathon beginning on the afternoon of the 29th and lasting ~1-2 days. |
|
My affiliation should be updated to NRL and my ORCID is 0000-0001-9642-6089. |
I wonder if this could be a responsibility of the proposed PyHC developer position? At least the first pass and then the initial review could be further evaluated by the community. |
standards.md
Outdated
| @@ -29,12 +35,12 @@ Definitions: | |||
| 5. **License**: Projects must provide a license. Projects should use permissive licenses for open source scientific software (e.g., the BSD 2-clause, BSD 3-clause, or BSD+Patent licenses). Copyleft licenses such as GPL are not recommended and OSI-approved permissive licenses are recommended. | |||
| 6. **Version control**: All code must use version control. It is strongly recommended that projects make use of a distributed version control system (e.g., git). | |||
| 7. **Coding Style**: Projects must adopt the basic style recommendations of [PEP 8](https://www.python.org/dev/peps/pep-0008/) and static analysis tools should be used to identify deviations from the basic style recommendations (e.g. pylint, flake8, pycodestyle). | |||
| 8. **Documentation**: All functions, classes, and modules must have documentation strings (docstrings) provided in a standard [conventions](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | |||
| 9. **Testing**: Stable packages must provide unit tests of individual components (e.g. functions, classes) as well as integration tests that test the interaction between components that covers most of the code. Testing coverage should be measured. Automated testing is recommended, in which tests are run before any code is merged. System[link] and acceptance[link] testing are also recommended. | |||
| 8. **Documentation**: All functions, classes, and modules in the public-facing application programming interface (API) must have documentation strings (docstrings) provided in a standard [convention](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | |||
There was a problem hiding this comment.
I'm not sure I understand this change. This suggests that non public-facing functions do not need doc strings. How is a contributor supposed to interact with developer code that is not documented?
There was a problem hiding this comment.
Hm...good point. I've occasionally written short private helper functions/methods with no arguments that have their one-line docstring encoded in the function name, so the docstring doesn't end up adding any additional information. I'm also sometimes lax on docstrings for unstable temporary implementations that I know will not be lasting long. Perhaps we could say that all public facing functions/etc must have docstrings, and that private functions/etc should generally have docstrings?
There was a problem hiding this comment.
I think "all public facing functions/etc must have docstrings, and that private functions/etc should generally have docstrings" is a good compromise. I know there are non-public facing functions in SciPy that use internal comments instead of docstrings when the functions are never supposed to be interacted with externally.
There was a problem hiding this comment.
I don't believe docstrings and comments are always needed if the code is self-explanatory. If the function has a clear "verb" that explains by itself what's the intention then comments and documentation may, not only, be redundant but also it can make the project harder to maintain - as it may be forgotten and never updated even making the documentation misleading! (and that's worse than no documentation at all!!)
So, rather than force people to have docstrings I would prefer to encourage good practices when naming functions, variables, etc. (which we don't have anything on that respect and I believe it's as important, or more, than documentation in itself). Chapter 2 of Clean Code is a good text about why we should encourage it more.
In any case, I agree with @namurphy's must/should rewrite.
There was a problem hiding this comment.
Certainly requiring full numpydoc docstrings on all private functions is overkill. +1 on @aburrell 's suggestion
There was a problem hiding this comment.
I'm fine with going with the "should". I don't really like the idea of encouraging "second-class" functions that are more poorly documented than others but I understand the concern here. I do know that for sunpy there are many important non-public facing functions that need good documentation for developers to interact with them!
standards.md
Outdated
| 8. **Documentation**: All functions, classes, and modules must have documentation strings (docstrings) provided in a standard [conventions](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | ||
| 9. **Testing**: Stable packages must provide unit tests of individual components (e.g. functions, classes) as well as integration tests that test the interaction between components that covers most of the code. Testing coverage should be measured. Automated testing is recommended, in which tests are run before any code is merged. System[link] and acceptance[link] testing are also recommended. | ||
| 8. **Documentation**: All functions, classes, and modules in the public-facing application programming interface (API) must have documentation strings (docstrings) provided in a standard [convention](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | ||
| 9. **Testing**: Stable packages must provide unit tests of individual components (e.g. functions, classes) as well as integration tests that test the interaction between components that covers most of the code. Testing coverage should be measured. Automated testing is recommended, in which tests are run before any code is merged. System and acceptance testing are also recommended. | ||
| 10. **Dependencies**: Projects should import the minimum number of packages necessary. Adding new dependencies should be a __considered__ decision. | ||
| 11. **Python 3**: All packages must be compatible or work towards being compatible with Python 3. Providing ongoing support for Python 2 is not recommended as the end of life for Python 2 is January 1, 2020 (see [PEP 373](https://www.python.org/dev/peps/pep-0373/)). |
There was a problem hiding this comment.
I would update this to state that Python 2 has reached its end of life and would rephrase to be more forceful in that all packages MUST support Python 3.
There was a problem hiding this comment.
Agreed completely! This slipped my mind because I was making these edits in 2019.
There was a problem hiding this comment.
- python3 may be redundant with 12. deprecation. It may now (2020) finally be sufficient to just omit mention of python2 and just mention NEP29.
Good point! I had thought I had emailed the list after I originally made the PR, but I forgot to... I put my notes from today's discussion in the commit log, though I should have put it as a comment here. For discussions at meetings/telecons, we should make sure that notes are posted in the corresponding PR.
|
standards.md
Outdated
| @@ -29,12 +35,12 @@ Definitions: | |||
| 5. **License**: Projects must provide a license. Projects should use permissive licenses for open source scientific software (e.g., the BSD 2-clause, BSD 3-clause, or BSD+Patent licenses). Copyleft licenses such as GPL are not recommended and OSI-approved permissive licenses are recommended. | |||
| 6. **Version control**: All code must use version control. It is strongly recommended that projects make use of a distributed version control system (e.g., git). | |||
| 7. **Coding Style**: Projects must adopt the basic style recommendations of [PEP 8](https://www.python.org/dev/peps/pep-0008/) and static analysis tools should be used to identify deviations from the basic style recommendations (e.g. pylint, flake8, pycodestyle). | |||
| 8. **Documentation**: All functions, classes, and modules must have documentation strings (docstrings) provided in a standard [conventions](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | |||
| 9. **Testing**: Stable packages must provide unit tests of individual components (e.g. functions, classes) as well as integration tests that test the interaction between components that covers most of the code. Testing coverage should be measured. Automated testing is recommended, in which tests are run before any code is merged. System[link] and acceptance[link] testing are also recommended. | |||
| 8. **Documentation**: All functions, classes, and modules in the public-facing application programming interface (API) must have documentation strings (docstrings) provided in a standard [convention](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | |||
There was a problem hiding this comment.
I think "all public facing functions/etc must have docstrings, and that private functions/etc should generally have docstrings" is a good compromise. I know there are non-public facing functions in SciPy that use internal comments instead of docstrings when the functions are never supposed to be interacted with externally.
| Amended on 06-Jan-2020 by (in alphabetical order) | ||
|
|
||
| **N. Murphy** (CfA), | ||
|
|
There was a problem hiding this comment.
A. G. Burrell (NRL). Also, A. Halford is now at NASA GSFC.
minimum Python version observationsI have required Python 3.6 for over a year on almost 100% of my packages (aggregate millions of users across geoscience, life sciences, aerospace, etc) with only a handful of complaints, including from incorporation into Linux distributions, offline/proprietary/OEM users and others not captured by PyPi.
Although there are limitations on relevance of PyPi download stats (does not account for As Jeff also noted, there are increasing penalties to supporting Python 3.5. If one looks at and assumes sufficient representation of PyPi stats, Python 3.5 may be a small audience for a growing number of PyHC packages. In my opinion the main penalties of Python 3.5 include:
Python 3.7 brings dataclasses and better UTF8 locale support among other benefits |
| 14. **Binaries**: Binary files should be added to the package repository only when necessary in order to keep packages as light as possible. Jupyter notebooks can be binary files and should not be committed to the package repository but can be provided in other repositories. | ||
| 15. **Code of conduct**: Each project must adopt a code of conduct that is compatible with the [Contributor Covenant](https://www.contributor-covenant.org) and make it publicly available. | ||
|
|
||
| 12. **Deprecation Policy** (in accordance with [NEP 29](https://numpy.org/neps/nep-0029-deprecation_policy.html)): Each project should support (1) all minor versions of Python released 42 months prior to the project, and at minimum the two latest minor versions; and (2) all minor versions of NumPy released in the 24 months prior to the project, and at minimum the last three minor versions. In ``setup.py``, the ``python_requires`` variable should be set to the minimum supported version of Python. All supported minor versions of Python should be in the test matrix and have binary artifacts built for the release. Minimum Python and NumPy version support should be adjusted upward only on major and minor releases, and never on a patch release. |
There was a problem hiding this comment.
...In setup.py or setup.cfg, the python_requires ...
Although perhaps too technical for the standards document, in my opinion setup.cfg should be used as much as possible to allow better tool introspection without needing to install. For security, best practices, etc.
There was a problem hiding this comment.
Agree with @scivision. We should encourage to keep that metadata on setup.cfg or pyproject.toml.
|
Re: Black, I think it may be too much to have in PyHC standards in general, I have run into its corner cases that make code less readable. Also the whole defaulting to double quotes thing is problematic to me, although I would rather have a statement encouraging setup.cfg over setup.py if we're going to get technical :-) |
|
re: Docs |
There was a problem hiding this comment.
Looks good @namurphy! Thanks!!
Re Black: I'm with @scivision - I wouldn't like to enforce it anywhere. Most of the times I've used I've found myself unhappy afterwards. I don't think that having "perfect" style at costs of readability has any advantages. Instead of suggesting an automatic linter I would keep the suggestion as we have on 7.
Re "grading packages": I'm happy to offer volunteer to review systems and packages... but as my free time allows me to go through them. So slowly. Additionally to the points you've linked, there's also the Best practice badge which is a big form that it needs to be filled up. I could use such questionaire to review the different projects (like I did with SunPy a while ago).
My affiliation hasn't changed, and my ORCID is: 0000-0003-0784-6909
standards.md
Outdated
| @@ -29,12 +35,12 @@ Definitions: | |||
| 5. **License**: Projects must provide a license. Projects should use permissive licenses for open source scientific software (e.g., the BSD 2-clause, BSD 3-clause, or BSD+Patent licenses). Copyleft licenses such as GPL are not recommended and OSI-approved permissive licenses are recommended. | |||
| 6. **Version control**: All code must use version control. It is strongly recommended that projects make use of a distributed version control system (e.g., git). | |||
| 7. **Coding Style**: Projects must adopt the basic style recommendations of [PEP 8](https://www.python.org/dev/peps/pep-0008/) and static analysis tools should be used to identify deviations from the basic style recommendations (e.g. pylint, flake8, pycodestyle). | |||
| 8. **Documentation**: All functions, classes, and modules must have documentation strings (docstrings) provided in a standard [conventions](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | |||
| 9. **Testing**: Stable packages must provide unit tests of individual components (e.g. functions, classes) as well as integration tests that test the interaction between components that covers most of the code. Testing coverage should be measured. Automated testing is recommended, in which tests are run before any code is merged. System[link] and acceptance[link] testing are also recommended. | |||
| 8. **Documentation**: All functions, classes, and modules in the public-facing application programming interface (API) must have documentation strings (docstrings) provided in a standard [convention](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | |||
There was a problem hiding this comment.
I don't believe docstrings and comments are always needed if the code is self-explanatory. If the function has a clear "verb" that explains by itself what's the intention then comments and documentation may, not only, be redundant but also it can make the project harder to maintain - as it may be forgotten and never updated even making the documentation misleading! (and that's worse than no documentation at all!!)
So, rather than force people to have docstrings I would prefer to encourage good practices when naming functions, variables, etc. (which we don't have anything on that respect and I believe it's as important, or more, than documentation in itself). Chapter 2 of Clean Code is a good text about why we should encourage it more.
In any case, I agree with @namurphy's must/should rewrite.
| 8. **Documentation**: All functions, classes, and modules must have documentation strings (docstrings) provided in a standard [conventions](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | ||
| 9. **Testing**: Stable packages must provide unit tests of individual components (e.g. functions, classes) as well as integration tests that test the interaction between components that covers most of the code. Testing coverage should be measured. Automated testing is recommended, in which tests are run before any code is merged. System[link] and acceptance[link] testing are also recommended. | ||
| 8. **Documentation**: All functions, classes, and modules in the public-facing application programming interface (API) must have documentation strings (docstrings) provided in a standard [convention](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | ||
| 9. **Testing**: Stable packages must provide unit tests of individual components (e.g. functions, classes) as well as integration tests that test the interaction between components that covers most of the code. Testing coverage should be measured. Automated testing is recommended, in which tests are run before any code is merged. System and acceptance testing are also recommended. |
There was a problem hiding this comment.
I believe we should add links to all the four types of tests that are mentioned here. I've just found the software testing fundamentals website which describe them quite well. Otherwise, people that haven't obtained any software development training wouldn't really understand the differences.
| 14. **Binaries**: Binary files should be added to the package repository only when necessary in order to keep packages as light as possible. Jupyter notebooks can be binary files and should not be committed to the package repository but can be provided in other repositories. | ||
| 15. **Code of conduct**: Each project must adopt a code of conduct that is compatible with the [Contributor Covenant](https://www.contributor-covenant.org) and make it publicly available. | ||
|
|
||
| 12. **Deprecation Policy** (in accordance with [NEP 29](https://numpy.org/neps/nep-0029-deprecation_policy.html)): Each project should support (1) all minor versions of Python released 42 months prior to the project, and at minimum the two latest minor versions; and (2) all minor versions of NumPy released in the 24 months prior to the project, and at minimum the last three minor versions. In ``setup.py``, the ``python_requires`` variable should be set to the minimum supported version of Python. All supported minor versions of Python should be in the test matrix and have binary artifacts built for the release. Minimum Python and NumPy version support should be adjusted upward only on major and minor releases, and never on a patch release. |
There was a problem hiding this comment.
Agree with @scivision. We should encourage to keep that metadata on setup.cfg or pyproject.toml.
|
Please add me to the list. Orcid is 0000-0001-6673-3432 |
There was a problem hiding this comment.
Can we also (maybe after this is approved) do a wrap of long lines so that the diff is a little easier to read? I know there will always be weirdness with that (rewrapping when something is longer or shorter) but this was hard...hopefully doing so would also provide enough context so that changing numbers (by inserting new guidelines) doesn't look like something completely new.
| @@ -29,12 +35,12 @@ Definitions: | |||
| 5. **License**: Projects must provide a license. Projects should use permissive licenses for open source scientific software (e.g., the BSD 2-clause, BSD 3-clause, or BSD+Patent licenses). Copyleft licenses such as GPL are not recommended and OSI-approved permissive licenses are recommended. | |||
| 6. **Version control**: All code must use version control. It is strongly recommended that projects make use of a distributed version control system (e.g., git). | |||
| 7. **Coding Style**: Projects must adopt the basic style recommendations of [PEP 8](https://www.python.org/dev/peps/pep-0008/) and static analysis tools should be used to identify deviations from the basic style recommendations (e.g. pylint, flake8, pycodestyle). | |||
| 8. **Documentation**: All functions, classes, and modules must have documentation strings (docstrings) provided in a standard [conventions](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | |||
| 9. **Testing**: Stable packages must provide unit tests of individual components (e.g. functions, classes) as well as integration tests that test the interaction between components that covers most of the code. Testing coverage should be measured. Automated testing is recommended, in which tests are run before any code is merged. System[link] and acceptance[link] testing are also recommended. | |||
| 8. **Documentation**: All functions, classes, and modules in the public-facing application programming interface (API) must have documentation strings (docstrings) provided in a standard [convention](https://www.python.org/dev/peps/pep-0257/) (e.g. [numpydoc](https://numpydoc.readthedocs.io/en/latest/format.html)). Functions, classes, and modules that are not public-facing should generally have docstring to enable developers to maintain them. Docstrings must describe the code’s purpose, describe all inputs and outputs, and provide examples. High level documentation must also be provided as guides, tutorials, and developer docs. Documentation must be provided in version control with the code and be made available online in a readable form. | |||
There was a problem hiding this comment.
"should generally have docstring" --should be plural.
| 15. **Code of conduct**: Each project must adopt a code of conduct that is compatible with the [Contributor Covenant](https://www.contributor-covenant.org) and make it publicly available. | ||
|
|
||
| 11. **Python 3**: All packages must be compatible with Python 3. Providing ongoing support for Python 2 is not recommended as the end of life for Python 2 was January 1, 2020 (see [PEP 373](https://www.python.org/dev/peps/pep-0373/)). | ||
| 12. **Deprecation Policy** (in accordance with [NEP 29](https://numpy.org/neps/nep-0029-deprecation_policy.html)): Each project should support (1) all minor versions of Python released 42 months prior to the project, and at minimum the two latest minor versions; and (2) all minor versions of NumPy released in the 24 months prior to the project, and at minimum the last three minor versions. In ``setup.py``, the ``python_requires`` variable should be set to the minimum supported version of Python. All supported minor versions of Python should be in the test matrix and have binary artifacts built for the release. Minimum Python and NumPy version support should be adjusted upward only on major and minor releases, and never on a patch release. |
There was a problem hiding this comment.
Can't this just be "projects should comply with NEP 29"?
|
On line wrapping, I generally find the best thing is one line per sentence. It (ab)uses the git diff algorithm to highlight changes on the sentence by sentence basis. |
|
This is what I do and it even works with diffing LaTeX documents.
…On Fri, Feb 7, 2020, 11:00 Stuart Mumford ***@***.***> wrote:
On line wrapping, I generally find the best thing is one line per
sentence. It (ab)uses the git diff algorithm to highlight changes on the
sentence by sentence basis.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#16?email_source=notifications&email_token=ADB5MVM76IGJIIWJTBJN5HDRBWASJA5CNFSM4J4QTEJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELDQUAI#issuecomment-583469569>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADB5MVMOW4QKTEFFOOBOFN3RBWASJANCNFSM4J4QTEJA>
.
|
|
The HDEE Step-2 are due 1 July. Is it expected that the PyHC standards will be updated before the due date? |
Co-authored-by: Will Barnes <[email protected]>
This pull request is to include changes to the community standards based on the discussion at the PyHC meeting from November 2019. We probably still need to update the author list to reflect the people who were in attendance in Boulder. We'll need to review this during a PyHC telecon before we accept this, which will probably be in January 2020.
Here are the notes from the meeting back in November: