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.

Sign up for GitHub

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

Jump to bottom

Issue #784 -- Dynamic author list #787

Merged
merged 7 commits into from
Mar 20, 2016
Merged

Issue #784 -- Dynamic author list #787

merged 7 commits into from
Mar 20, 2016

Conversation

jbarnoud
Copy link
Contributor

Fixes #784

Changes made in this Pull Request:

  • Add Lennard van der Feltz in package/AUTHORS
  • setup.py reads AUTHORS and write authors.py that get imported by __init__.py to expose __authors__.
  • Sphinx uses __authors__ for its author list.

The order of the authors is not perfect. Except for the two first authors and the last author, the authors are sorted alphabetically by their last name. Yet, last names are difficult to define; the last word of the name is used as last name. It causes an issue with "Christian Beckstein (logo)" because of the "(logo)", also "Manel Nuno Melo" was sorted at "Nuno" and is now sorted as "Melo".

The code is repeated between package/setup.py and testsuite/setup.py. I do not like it much but I do not see how to avoid that easily.

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

jbarnoud added 6 commits March 20, 2016 01:20
@jbarnoud
Add missing Lennard van der Felzt in AUTHORS
c7a1ba0
@jbarnoud
setup.py writes authors.py in package
4e5d502 
A list of authors usable from python is generated from the AUTHORS file
by 'package/setup.py'. That list allows to maintain a single list of
authors instead of one list in AUTHORS and one list in the sphinx
documentation.

MDAnalysis.__init__ imports authors.py to give access to
MDAnalysis.__authors__.
@jbarnoud
Sphinx uses __authors__
1c2e62f 
The list of authors used by sphinx is the one in authors.py.
@jbarnoud
Generate authors.py from AUTHORS for the tests
2937b74
@jbarnoud
Test that __authors__ is defined
5b1aadf
@jbarnoud
Update CHANGELOGs for __authors__
eee3cee
richardjgowers
richardjgowers reviewed Mar 20, 2016
View reviewed changes
package/MDAnalysis/__init__.py
@@ -140,6 +140,7 @@
import warnings

from .version import __version__
from .authors import __authors__
Copy link
Member

Choose a reason for hiding this comment

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

So if setup is ran from a location where you don't have write permission (is this possible?), this file won't be generated. So would it be safer to wrap this in a try/except block just so this never stops the entire module working one day?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Fixed.

@jbarnoud
Wrap the imports of authors.py
a68eedf 
MDAnalysis should not fail if authors.py cannot be generated and
therefore imported. If authors.py is not present, then __authors__ is an
empty list.
@richardjgowers richardjgowers self-assigned this Mar 20, 2016
richardjgowers added a commit that referenced this pull request Mar 20, 2016
@richardjgowers
Merge pull request #787 from jbarnoud/issue-784-dynamic-authors
3de4db6 
Issue #784 -- Dynamic author list
@richardjgowers richardjgowers merged commit 3de4db6 into MDAnalysis:develop Mar 20, 2016
@richardjgowers richardjgowers added this to the 0.15.0 milestone Mar 20, 2016
jbarnoud added a commit to jbarnoud/mdanalysis that referenced this pull request Mar 20, 2016
@jbarnoud
Remove relative imports introduced in MDAnalysis#787
c7a1593 
We prohibit relative imports in the tests (see MDAnalysis#189). When adding tests
for the dynamic author list (MDAnalysis#787), I introduced some such relative
imports.
@jbarnoud jbarnoud mentioned this pull request Mar 20, 2016
Merged
richardjgowers added a commit that referenced this pull request Mar 20, 2016
@richardjgowers
Merge pull request #791 from jbarnoud/issue-784-dynamic-authors
2cc1cc3 
Remove relative imports introduced in #787
@orbeckst
Copy link
Member

@jbarnoud , this commit seems to badly break the 3.3 tests right at the beginning, see e.g. https://travis-ci.org/MDAnalysis/mdanalysis/jobs/117197996

  Running setup.py (path:/tmp/pip-gcs5l6-build/setup.py) egg_info for package from file:///home/travis/build/MDAnalysis/mdanalysis/package
    Running command python setup.py egg_info
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-gcs5l6-build/setup.py", line 424, in <module>
        dynamic_author_list()
      File "/tmp/pip-gcs5l6-build/setup.py", line 392, in dynamic_author_list
        name = line.strip()[2:].strip().decode('utf-8')
    AttributeError: 'str' object has no attribute 'decode'

@jbarnoud
Copy link
Contributor Author

Ghraaa! I omitted to run the tests on python 3. I'll work on this.

If anybody has an idea why Travis crashes on the python 3 builds, it would be nice to fix. I had a look but I do not know what happens.

jbarnoud added a commit to jbarnoud/mdanalysis that referenced this pull request Mar 21, 2016
@jbarnoud
Dynamic author list works on python 3
0de57c8 
The dynamic author list introduced in MDAnalysis#787 did not work on python 3
because of the way encoding was handled. This commit fixes the handling
of file encodings by using the codecs library.
@jbarnoud jbarnoud mentioned this pull request Mar 21, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@richardjgowers richardjgowers

Labels
None yet
Projects
None yet
Milestone
0.15.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jbarnoud @orbeckst @richardjgowers