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

Rework Abins docs: input data formats info split out to another page #38588

Merged
merged 5 commits into from
Jan 15, 2025

Conversation

ajjackson
Copy link
Contributor

Migrated #38518 from external fork to a branch on mantidproject; see initial review there. Hopefully this will make it easier to get useful package artifacts from CI and verify that the docs are ok.

Description of work

Add a page to Concepts docs

Summary of work

Break out some information from Abins / Abins2D Algorithm docs and expand with a bit more information in a new Concepts doc, linked from the Algorithm docs.

Purpose of work

resolves #38352

A bit more documentation is needed to clarify how isotope data should be used with CASTEP calculations fed to Abins. If we put all the code-specific "gotchas" into the Algorithm docs it can

a) get a bit unwieldy;
b) lead to a lot of duplication between Abins and Abins2D docs.

The new page also provides a natural home for a quick-reference table of supported data formats.

Report to: Pablo Gila-Herranz

To test:

Docs should build. Probably a good idea to look at them too: make sure the new page appears in the Concepts index as well as the Abins Algorithm pages.


Reviewer

Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.

Code Review

  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards?
  • Are the unit tests small and test the class in isolation?
  • If there is GUI work does it follow the GUI standards?
  • If there are changes in the release notes then do they describe the changes appropriately?
  • Do the release notes conform to the release notes guide?

Functional Tests

  • Do changes function as described? Add comments below that describe the tests performed?
  • Do the changes handle unexpected situations, e.g. bad input?
  • Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

Gatekeeper

If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.

@ajjackson ajjackson changed the title Docs concepts abins data Rework Abins docs: input data formats info split out to another page Jan 10, 2025
@sf1919 sf1919 added this to the Release 6.12 milestone Jan 10, 2025
@SilkeSchomann
Copy link
Contributor

I have tried to build a package from this branch, but it fails an Abins test: python.scripts.AbinsPowderCalculatorTest.AbinsPowderCalculatorTest

@ajjackson
Copy link
Contributor Author

ajjackson commented Jan 13, 2025

That's the scipy-related one from last week. Doesn't make sense that the checks would pass here but not there; perhaps that builder doesn't have the latest Scipy so fails on the updated test data? (Scipy changed some remote decimal places of the reduced Planck constant...)

Is there some kind of "clean environment" build option that would update the Conda packages?

@ajjackson
Copy link
Contributor Author

Perhaps that test should be modified to use a lower tolerance if it detects an old Scipy 🤔

@ajjackson
Copy link
Contributor Author

I can get the docs to build on Linux, but the new entry does not appear on the Concepts index. I tried changing to a section label that doesn't contain spaces, but it doesn't seem to have helped.

I noticed that the table header still looked a bit off, that should be fixed now.

Even without spaces in the label, I am having trouble getting the new
file to appear in the concepts index.
This category seems to be required for the new page to show up in
concepts index.

Also add a little contents section to the top of the file.
@ajjackson ajjackson force-pushed the docs-concepts-abins-data branch from 62db159 to b8b7f9b Compare January 15, 2025 13:45
Copy link
Contributor

@SilkeSchomann SilkeSchomann left a comment

Choose a reason for hiding this comment

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

I can confirm that the table header on the AbinsDataFormats page is now as expected and there is an entry for AbinsDataFormats on the concepts page. The links on the Abins-v1 and Abins2D-v1 pages still work 👍🏻

@robertapplin robertapplin self-assigned this Jan 15, 2025
@robertapplin robertapplin enabled auto-merge (squash) January 15, 2025 14:23
@robertapplin robertapplin merged commit 9884255 into main Jan 15, 2025
10 checks passed
@robertapplin robertapplin deleted the docs-concepts-abins-data branch January 15, 2025 14:57
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.

Abins Not Identifying Deuterium Species
4 participants