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

Rewrite PyDPF-Post doc to PyAnsys library style guidelines #785

Merged
merged 24 commits into from
Jan 31, 2025

Conversation

PipKat
Copy link
Member

@PipKat PipKat commented Jan 9, 2025

Ready for review.

Implemented cards on index pages of "Getting started" and "User guide" sections and did an overall review of these sections and the "Examples" and "Contribute" sections. I will do an overall review of the "API reference" section after it uses the new styling, which was recently implemented in PyDPF-Core.

@RobPasMue There are two pages in the HTML artifact where images and a heading for a subsequent section run together. One page is "Brief demo" ( demo.rst) in the "Getting started" section. The other page is "Plot results" (plotting.rst) in the "User guide" section. I changed the directive from figure to image thinking that might fix it. However, it didn't. Do you know how I can get the next section on a new line? Copilot said to put a blank line after the block. However, there are two blank lines already there.

@PipKat PipKat changed the title Rewriting PyDPF-Post doc to PyAnsys library style guidelines Rewrite PyDPF-Post doc to PyAnsys library style guidelines Jan 9, 2025
Copy link

codecov bot commented Jan 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.11%. Comparing base (14850f7) to head (02b874b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #785   +/-   ##
=======================================
  Coverage   85.11%   85.11%           
=======================================
  Files          52       52           
  Lines        5208     5208           
=======================================
  Hits         4433     4433           
  Misses        775      775           

@PipKat PipKat marked this pull request as ready for review January 10, 2025 15:21
@RobPasMue
Copy link
Member

@PipKat - looks like my change didn't work. Maybe @Revathyvenugopal162 can look into it a bit more in depth?

@PipKat
Copy link
Member Author

PipKat commented Jan 14, 2025

@PProfizi and @cbellot000 I think you guys can review this PR, and we can get it merged. I can work with Revathy later to get the spacing problem of images and subsequent subheadings appearing on the same line if the images are left-aligned. In the meantime, I'll center align the images referenced in the description. NOTE: The images in the examples are left-aligned, so there must be a way to do a left alignment in reStructuredText files!

Also, I didn't do any editing of the "API reference" section. I assumed you'd be working with @moe-ad to update this section. I'd be happy to do a complete doc review of this section after reworking it.

@PipKat PipKat requested review from PProfizi and cbellot000 January 14, 2025 15:42
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
Copy link
Contributor

@PProfizi PProfizi left a comment

Choose a reason for hiding this comment

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

Hi @PipKat, finished my review. Thanks for the huge amount of work this must have been!

@PipKat
Copy link
Member Author

PipKat commented Jan 28, 2025

@PProfizi I need you or someone else to approve this PR to be able to merge it! Thanks!

@PipKat PipKat enabled auto-merge (squash) January 31, 2025 17:52
@PipKat PipKat merged commit 7553885 into master Jan 31, 2025
33 of 34 checks passed
@PipKat PipKat deleted the doc/reditorial_review branch January 31, 2025 18:08
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.

None yet

4 participants