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

New contributing guidelines #646

Merged
merged 7 commits into from
May 30, 2020
Merged

New contributing guidelines #646

merged 7 commits into from
May 30, 2020

Conversation

ashtonmv
Copy link
Contributor

@ashtonmv ashtonmv commented May 7, 2020

This is intended as a draft, which hopefully others can also contribute to.

Closes #643, but now that pyiron is public on github and has ~15 contributors, it might be a good time to expand the contributing guidelines. Feel free to remove/replace 100% of what I wrote, since you guys are the core maintainers.

I noticed that @jan-janssen already had some stuff related to this in docs/source/developers.rst, so that's what I've built on. Nearly everything I changed is in CONTRIBUTING.rst, which developers.rst imports from for the online documentation.

To start the discussion, I have a few points/questions:

  1. What's the best way for people to ask us questions (email, issues page, slack channel, etc.)? If someone wants to "join the organization", what should they do? Do we even want that?
  2. Is it worth the effort to enforce issue/pull request templates like these? That could just be despotic while we're still small, but as the repository grows it might become very helpful.
  3. The sections "The structure of pyiron" and "The philosophy of pyiron" should probably be filled in by someone who knows more about the code than I do. IMO, the more graphically we can show pyiron's code structure the better, like the uml diagram in build_docs_on_master.sh but simpler. There's a lot of multi-level inheritance in pyiron and a naive new contributor (read: myself) will have difficulty following these around at first.

@coveralls
Copy link

coveralls commented May 7, 2020

Pull Request Test Coverage Report for Build 4780

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1584 unchanged lines in 19 files lost coverage.
  • Overall coverage increased (+0.2%) to 57.54%

Files with Coverage Reduction New Missed Lines %
pyiron/atomistics/structure/sparse_list.py 1 66.46%
pyiron/atomistics/job/atomistic.py 4 68.84%
pyiron/lammps/structure.py 6 74.59%
pyiron/base/job/jobtype.py 9 83.33%
pyiron/atomistics/volumetric/generic.py 10 94.15%
pyiron/base/generic/hdfio.py 12 74.33%
pyiron/dft/job/generic.py 16 61.0%
pyiron/lammps/potential.py 16 75.19%
pyiron/atomistics/master/phonopy.py 27 0%
pyiron/lammps/base.py 29 79.38%
Totals Coverage Status
Change from base Build 4456: 0.2%
Covered Lines: 12203
Relevant Lines: 21208

💛 - Coveralls

@liamhuber liamhuber removed their request for review May 7, 2020 14:45
@jan-janssen jan-janssen added the enhancement Category: New feature or request label May 8, 2020
Copy link
Member

@sudarsan-surendralal sudarsan-surendralal left a comment

Choose a reason for hiding this comment

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

This looks great! Should we also have a markdown version CONTRIBUTING.md on github?

CONTRIBUTING.rst Outdated
Comment on lines 54 to 63
.. _What should I know before I get started?:
What should I know before I get started?
========================================

The structure of pyiron
-----------------------

The principles of pyiron
------------------------

Choose a reason for hiding this comment

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

I think the structure & principles of pyiron is something that will change drastically in the future based on the discussions we had earlier. For the time being, we could save these lines as a comment and work on it once the new structure is finalized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks great! Should we also have a markdown version CONTRIBUTING.md on github?

I'm happy to also make a markdown version, but it seems the rst file also renders automatically online. Everything else was in rst for the sphinx docs, so that's why I left it that way.

I think the structure & principles of pyiron is something that will change drastically in the future based on the discussions we had earlier. For the time being, we could save these lines as a comment and work on it once the new structure is finalized.

sounds good 👍

CONTRIBUTING.rst Outdated
If you don't already have the dependencies installed in your environment,
```
cd pyiron
pip install .

Choose a reason for hiding this comment

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

Wouldn't it be more consistent to install all the packages through conda-forge first and then overwrite the python path to use the your local branch of pyiron?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah doing it with conda does seem more elegant, but I admit I am sometimes confused by the interaction between conda and $PYTHONPATH.

Maybe conda develop is the way to go? e.g.

conda create --name pyiron_dev
conda activate pyiron_dev
conda install -c conda-forge conda-build pyiron
conda remove --force pyiron  # leaves dependencies untouched
conda develop /path/to/git/installed/pyiron  # adds git folder to site-packages/conda.pth file

* Limit the first line to 72 characters or less
* Reference issues and pull requests liberally after the first line
* When only changing documentation, include [ci skip] in the commit title
* Consider starting the commit message with an applicable emoji:

Choose a reason for hiding this comment

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

For pycharm users, you can install the emoji plugin to add emojis for your commit messages: 👨‍🎨 🐎 🍰

Python styleguide
-----------------

Please follow `PEP8 conventions`_ for all python code added to pyiron. Pull

Choose a reason for hiding this comment

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

Maybe IDEs like pycharm could be recommended

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should add a section like "Tips and Tricks" where we include this kind of recommendation

@stale
Copy link

stale bot commented May 25, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added wontfix and removed wontfix labels May 25, 2020
@ashtonmv ashtonmv marked this pull request as ready for review May 30, 2020 15:04
@ashtonmv
Copy link
Contributor Author

My impression from yesterday was that we can merge this, with more changes likely to come someday. Is that right?

@ashtonmv ashtonmv merged commit 4af2e2f into master May 30, 2020
@ashtonmv ashtonmv deleted the contributing_guidelines branch May 30, 2020 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Category: New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

git installation instructions
6 participants