Skip to content

[WIP] Issue 993 update citation#1001

Closed
tylerjereddy wants to merge 3 commits intodevelopfrom
issue-993-update-citation
Closed

[WIP] Issue 993 update citation#1001
tylerjereddy wants to merge 3 commits intodevelopfrom
issue-993-update-citation

Conversation

@tylerjereddy
Copy link
Copy Markdown
Member

@tylerjereddy tylerjereddy commented Sep 23, 2016

Reference: #993

This PR aims to update the citation information in all pertinent package / documentation files. I only found two places in the sphinx docs where changes were needed, and I did those two changes and the README manually.

Doing the metadata (package file headers) manually would be insane, so I drafted a metadata citation injection and verification script / gist for the new citation.

The final output from running the above script:

Done; files changed: 172
QC check; num problematic files in full package: 0
problematic files: []

The WIP tag is placed on this PR because of the extremely large number of files changed in a semi-automated manner, thereby mandating a certain amount of caution / revision before merging.

For the metadata changes we might want to explicitly say to cite both papers rather than just listing both, which would just require a bit more string / file editing in the above script. May also be an argument for an extra newline in there [and / or URL info] depending on how picky you are.

@tylerjereddy tylerjereddy added this to the 0.16.0 milestone Sep 23, 2016
@orbeckst
Copy link
Copy Markdown
Member

orbeckst commented Sep 23, 2016

@tylerjereddy thanks for taking this on. If you are already working on the standard header then we should have a discussion what it should like. It is certainly not appropriate anymore that Naveen, Liz and I have such a prominent position in it.

@orbeckst
Copy link
Copy Markdown
Member

I opened #1002 for the broader discussion.

I would prefer to combine all comment header rewriting in one commit instead of two big ones. Can you script be modified to change other parts of the header as well, once we have agreed on something in #1002?

@tylerjereddy
Copy link
Copy Markdown
Member Author

Yeah, that's fine, it is just string / file line parsing.

@tylerjereddy tylerjereddy force-pushed the issue-993-update-citation branch from 07b4139 to 7f43f21 Compare September 28, 2016 12:00
@tylerjereddy
Copy link
Copy Markdown
Member Author

tylerjereddy commented Sep 28, 2016

I've updated the branch / PR with the latest proposal in #1002, though I haven't looked into @richardjgowers suggestion of the dynamic author list.

Note that my code parsed two less files this time (now that more changes are being made and the header is being detected from the start point):

['package/MDAnalysis/__init__.py', 'package/MDAnalysis/analysis/nuclinfo.py']

I couldn't quite figure out why based on a quick look.

@richardjgowers
Copy link
Copy Markdown
Member

I had a thought, this will create a merge conflict with every file in a PR that branched before this (notably #815 #797). It might make more sense to rerun the script up top after those have merged. As long as this gets done before the next release the ordering shouldn't matter too much?

@orbeckst
Copy link
Copy Markdown
Member

I think @richardjgowers #1001 (comment) raises a good point: the less we mess with the two big merges the better. (It might just go through without a hitch but in any case, these two big PRs will generate many new files whose headers will also have to be updated, so better do it all in one go.)

@tylerjereddy can we hold off and then you run the script once #797 and #815 are in?

@tylerjereddy
Copy link
Copy Markdown
Member Author

Sounds good. I've pushed the script to a repo because I couldn't push changes to the above anonymous gist. This is just a precaution in case the need to merge arises when I'm busy with relocation to the US ~mid-late December.

@kain88-de
Copy link
Copy Markdown
Member

superseded by #1088

@kain88-de kain88-de closed this Nov 27, 2016
@tylerjereddy tylerjereddy deleted the issue-993-update-citation branch November 27, 2016 18:48
@tylerjereddy tylerjereddy mentioned this pull request Nov 29, 2016
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants