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

Inline equations fix #947

Merged
merged 10 commits into from
Jan 24, 2024
Merged

Inline equations fix #947

merged 10 commits into from
Jan 24, 2024

Conversation

epolack
Copy link
Collaborator

@epolack epolack commented Jan 19, 2024

PR #946 introduced a bug where inline math was not rendered properly, as modifying :tex overwrote the default configuration. There were also minor style issues that I have corrected.

However, I have noticed signatures issues that I do not understand. Examples here and there.

Signatures done by hand are not taken into accounts, and some are duplicated.

@epolack epolack force-pushed the fix-docs branch 3 times, most recently from b0e5686 to 3693581 Compare January 19, 2024 14:29
@epolack epolack force-pushed the fix-docs branch 2 times, most recently from a951bcd to 4b97817 Compare January 22, 2024 10:02
@mfherbst
Copy link
Member

However, I have noticed signatures issues that I do not understand.

Have not looked at the details, but as far as I understand it with DocStringExtensions we don't need to do the signatures by hand any more, but maybe we use it wrongly.

@epolack
Copy link
Collaborator Author

epolack commented Jan 22, 2024

Oh, that's it! I did not noticed it was imported; thanks! I'll do a pass to remove the signatures we have.

@mfherbst
Copy link
Member

mfherbst commented Jan 22, 2024

Oh, that's it! I did not noticed it was imported; thanks! I'll do a pass to remove the signatures we have.

Thanks ! But don't do it blindly before checking that DocStringExtensions does not accidentially drop some information we have in the current signatures.

@epolack
Copy link
Collaborator Author

epolack commented Jan 22, 2024

Sure :)

@epolack
Copy link
Collaborator Author

epolack commented Jan 22, 2024

I played a bit, but I was not able to selectively replace some signatures and by default keep others.
It seems the @template does not merge the two.
And for the Model methods, I do not undertand why the @template does not happen.

So, maybe keep it that way for now; unless you have an idea…

(BTW, the CI fails catastrophically; no idea why.)

@epolack
Copy link
Collaborator Author

epolack commented Jan 23, 2024

The culprit was this change is Spglib. It seems to be an upstream issue.

@epolack What you did was not limiting the version from below, so 0.0.0 would be valid, which can be a problem 😄.
@mfherbst
Copy link
Member

@epolack Great catch ! Thanks very much !

@mfherbst mfherbst merged commit c92b59a into JuliaMolSim:master Jan 24, 2024
3 of 6 checks passed
@epolack epolack deleted the fix-docs branch February 19, 2024 13:27
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.

2 participants