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

Restructuring the documentation #1122

Merged
merged 33 commits into from
Mar 10, 2021
Merged

Restructuring the documentation #1122

merged 33 commits into from
Mar 10, 2021

Conversation

RosalynLP
Copy link
Contributor

Addresses #1121

@RosalynLP RosalynLP marked this pull request as draft March 2, 2021 15:33
@RosalynLP RosalynLP mentioned this pull request Mar 3, 2021
33 tasks
@RosalynLP RosalynLP mentioned this pull request Mar 3, 2021
26 tasks
@voisey
Copy link
Contributor

voisey commented Mar 4, 2021

@RosalynLP, these docs are looking niiiiiiiiiiiiiiiice

@RosalynLP
Copy link
Contributor Author

@RosalynLP, these docs are looking niiiiiiiiiiiiiiiice

We're getting there!

@RosalynLP RosalynLP marked this pull request as ready for review March 5, 2021 09:40
@RosalynLP
Copy link
Contributor Author

RosalynLP commented Mar 8, 2021

I think this is ready now @Zaharid @wilsonmr @siranipour @tgiani would be great if some of you could have a look and see what you think, any more structural suggestions etc. Caveat is that we intend to have an overview of the code after "getting started" to explain very loosely how everything links together - see @wilsonmr was assigned this in #656.

Also fixing the search bar broke Travis...

@Zaharid
Copy link
Contributor

Zaharid commented Mar 8, 2021

@RosalynLP @voisey Could I trouble you for a high level summary of what changed? Looking at the diff is not particularly easy...

@Zaharid
Copy link
Contributor

Zaharid commented Mar 8, 2021

@RosalynLP the error travis gives is

Error: bad character '>' in package version dependency 'sphinx_rtd_theme'

Perhaps you meant 'sphinx_rtd_theme >0.5'

@RosalynLP
Copy link
Contributor Author

@RosalynLP the error travis gives is

Error: bad character '>' in package version dependency 'sphinx_rtd_theme'

Perhaps you meant 'sphinx_rtd_theme >0.5'

Ok thank you!

@RosalynLP
Copy link
Contributor Author

RosalynLP commented Mar 9, 2021

@Zaharid

What has changed

  • Summaries have been added to each section where necessary, explaining what the section is about
  • The docs for code have been moved ahead of the theory section as we think this is more important for the user
  • vp guide has a new introduction and a "getting started" page explaining how to use vp on a v basic level, and the sections have been reordered and grouped together
  • examples section of vp guide has new table listing all the examples runcards, what they do and the corresponding tutorial if necessary
  • empty sections have been removed, e.g. Q&A, DGLAP collinear factorisation
  • Documentation section has been renamed "adding to documentation" and put at the end
  • Installing code 2 sections have been merged with explanations for when to do each one
  • rules and PR reviewing guidelines merged together
  • external code section moved out of getting started and below other code
  • tutorials have been grouped together by purpose
  • additional links have been added to point the user to the right sections
  • the page width has been increased for readability of tables and NNPDF logo added
  • some sections have been renamed to make it clearer what those bits of code do. We are open to suggestions for alternative names

Seee #1121 for a tick box list

@Zaharid
Copy link
Contributor

Zaharid commented Mar 10, 2021

As a comment n3fit/index.html seems somewhat out of date. Not sure where should I report that at this point.

@RosalynLP
Copy link
Contributor Author

As a comment n3fit/index.html seems somewhat out of date. Not sure where should I report that at this point.

Here is as good a place as any. Do you mean rather than describing it as a new model blah blah blah we just say it is the fitting code

@Zaharid
Copy link
Contributor

Zaharid commented Mar 10, 2021

I'd just remove a lot of that indeed.

@RosalynLP
Copy link
Contributor Author

RosalynLP commented Mar 10, 2021

@Zaharid lmk what you think of the last commit

@RosalynLP
Copy link
Contributor Author

I'm not entirely happy with the n3fit section... it explains the methodology of n3fit but is not very helpful for someone trying to understand the code. And the readme here https://github.com/NNPDF/nnpdf/tree/master/n3fit/src/n3fit is out of date. I think we could do with some sections on code structure, explaining how it all fits together and what the different bits do, and update the readme. @scarlehoff what are your thoughts on this? I am happy to do it but it would be good to have your help to look over what I've written.

But I think this is part of a broader issue we need to address of explaining the general code structure. We have decided to do a brief overview (assigned to @wilsonmr) but maybe we need something more for n3fit and validphys especially.

@scarlehoff
Copy link
Member

Sure, I'd be happy to help.
I should totally change/remove that readme, it's just the content of the PR when it was first merged...

@RosalynLP
Copy link
Contributor Author

OK amazing, although that is definitely work for a different PR than this.

@RosalynLP RosalynLP merged commit 3c633df into master Mar 10, 2021
@RosalynLP RosalynLP deleted the doc_reorder branch March 10, 2021 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants