angadhn / SpacecraftDynamics

Executable Textbook for Spacecraft Dynamics
http://www.angadhn.com/SpacecraftDynamics/
4 stars 10 forks source link

Lecture 5 #11

Closed thedukeofeelam closed 2 months ago

thedukeofeelam commented 3 months ago

Pull Request for my Lecture 5. Contains 1 ipynb and 7 png files.

review-notebook-app[bot] commented 3 months ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

angadhn commented 2 months ago

@Joosty can you help review this PR?

You could do this on here using ReviewNB but I now feel the below approach is better.

  1. clone the repository
  2. fetch this pr (using Pr number) as a branch. For example, in this case it would be using the number 11:
    git fetch origin pull/11/head:pr-11
  3. then checkout the branch.
  4. build the book locally and take a look at the output.
Joosty commented 2 months ago

Happy to, will start this evening and get back when I have some comments :)

Joosty commented 2 months ago

Issue 1: Images not loading

image

When I build the book images do not seem to be loading. I have looked in the ipynb file and see that the paths may not be fully set up. I have gone ahead and fixed this in my own version using the code below. While this is recognised in VSCode it still does not seem to load when the book is built. I will continue looking for a way to fix this.

image

angadhn commented 2 months ago

@Joosty Your task is only to review- not make fixes :-)

Joosty commented 2 months ago

Issue 2: \degree not rendering

The '\degree' method does not appear to be working. The best way of doing this is simply directly using the symbol ----> °

image

angadhn commented 2 months ago

Issue 2: \degree not rendering

The '\degree' method does not appear to be working. The best way of doing this is simply directly using the symbol ----> °

image

Actually ^\circ is better $^\circ$.

Joosty commented 2 months ago

Issue 3: Misc text issues

  1. Some brackets could be made to look nicer by using '\left[' and '\right]' in place of '\bigr' etc. Example: \left[\frac{-p}{u^2}\right]. Issue shown below: image

  2. '\ref' not working correctly. image

  3. Caption not rendering correctly image

  4. Unnessicary use of arrows on top of vectors, when I have spoken with @angadhn using \mathbf or \bf with no arrow is desired. image

A lot of these issues crop up across the file a few times so keep an eye out. If I see any other issues I will comment. Cheers and I hope this was helpful.

angadhn commented 2 months ago

Issue 3: Misc text issues

  1. Some brackets could be made to look nicer by using '\left[' and '\right]' in place of '\bigr' etc. Example: \left[\frac{-p}{u^2}\right]. Issue shown below:

image

  1. '\ref' not working correctly.

image

  1. Caption not rendering correctly

image

  1. Unnessicary use of arrows on top of vectors, when I have spoken with @angadhn using \mathbf or \bf with no arrow is desired.

image

A lot of these issues crop up across the file a few times so keep an eye out. If I see any other issues I will comment. Cheers and I hope this was helpful.

Brilliant review!

thedukeofeelam commented 2 months ago

Issue 3: Misc text issues

  1. Some brackets could be made to look nicer by using '\left[' and '\right]' in place of '\bigr' etc. Example: \left[\frac{-p}{u^2}\right]. Issue shown below: image
  2. '\ref' not working correctly. image
  3. Caption not rendering correctly image
  4. Unnessicary use of arrows on top of vectors, when I have spoken with @angadhn using \mathbf or \bf with no arrow is desired. image

A lot of these issues crop up across the file a few times so keep an eye out. If I see any other issues I will comment. Cheers and I hope this was helpful.

Thank you for all your support with this, I will try resolve them over the weekend!

Joosty commented 2 months ago

Just had a brief look over it, on first glance the images are not working still - maybe worth building yourself to check if this is working.

In the past, I have used the method shown in the screenshot to import the images. This involves giving the path to the image in question. This may be worth a try. image

I'll follow up if I find any other issues.

Cheers, Joost

angadhn commented 2 months ago

Looking very good, @thedukeofeelam- thanks! I think there are just two things to resolve.

  1. fixing images as mentioned by £Joosty.
  2. I see extra $ signs at the end here. image

@Joosty, I'll sign off with your approval once these fixes are made by @thedukeofeelam .

angadhn commented 2 months ago

@thedukeofeelam @Joosty Please ignore my above comment. I will merge this PR and make the minor fixes.

Please do the needful on the other PR asap, @thedukeofeelam. Thanks! This is all great stuff and I hope you feel you are learning something new and potentially valuable from this.

angadhn commented 2 months ago

Thanks again for your efforts, @thedukeofeelam and @Joosty. You can see the pages over here for this PR and the fruits of your labour!

angadhn commented 2 months ago

@Joosty just adding you in the reviewer list so we have a formal trail of your being involved for good measure. A bit late with doing that- sorry.