Closed nozzington closed 2 months ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
@thedukeofeelam can you please help review this PR?
@nozzington, lol it must be annoying hearing from me again, I'll keep this short.
1) Your level 1 heading is currently "#Lecture 3", replace this with "#nameOfLecture". Here's the textbook so far, make sure your new heading is not something that already exists on there: https://www.angadhn.com/SpacecraftDynamics/orbital-mechanics/Lecture4/Lecture4.html
2) Insert a cell for credits just like in the other lecture:
3) Add "In this lecture we aim to cover the following topics:" before the contents table 4) Code your contents table this way:
5) And ensure, you have this piece of code before each of your level 2 headings. This ensures the hyperlinks work as intended.
6) The main equations (which will require an equation number) must be coded this way:
7) Do you really want this to be red? I think for consistency's sake, leave it in black.
If you want to make it clearer that the red part is coming from the previous equation, then you can reference that equation within the text body like this: "The velocity vector (Equation {eq}L3_xyz
) is crossed with the specific angular momentum vector, $\mathbf{h}$ as here:" or something along that line.
8) Brackets may be made to look nicer using ChatGPT:
*is that = sign necessary there?
is turned into:
9) If my memory server right, this should not be in bold:
10) Likewise, check if this is correct? Maybe one one the e's should not be in bold?
11) The section on the equation of the orbit gets rendered this way:
This must be due to the spacing of the equations in your ipynb. Ensure it's all neat and perhaps has a spacing before after the double dollars. Tbh, you are going to turn most of the equations using point (6).
12) Elliptic Orbits figure does not get rendered on the HTML page:
Code it this way:
13) Use of colours, I'll let @angadhn decide on this. Maybe you can dump this section on what each letter means inside the figure caption.
14) The next tiny section on key parameters renders on HTML this way:
Again, the dollars pop up because you didn't leave spaces before the equations on two of them:
But reminder, you will be formatting the equations as noted in point (6).
15) Your next section: same issues as previously discussed:
16) Since Vis Viva is pretty vital, maybe you can make a reference to the equation here:
"The Vis Viva equation ({eq}L3_xyz
) is a crucial tool in mission planning for calculating velocities at various points in an orbit."
I just realised, the equation referencing gets rendered in my comments above (points 7 and 15). But the code behind referencing an equation looks like this:
@nozzington is this ready for review again by @thedukeofeelam and me?
@angadhn I believe Noah is still working on it - he's made some updates and has called a commit for those changes.
@nozzington, here is a PDF of the HTML page. I've added a couple of notes on there too. Essentially, work your way through yesterday's review points (and read the few points in red on the PDF too).
this should now be ready for review, if there are any issues I can fix again tonight
this should now be ready for review, if there are any issues I can fix again tonight
Alrighty. I am going to review them again in a bit and let you know before you start working on it again tonight.
@nozzington
1) Bolden this 'c'
2) Rogue $$
That $$ needs to be replaced with: ``` as here:
3) This section renders this way:
Leave a space like this in the markup to address that:
4) Brackets around the theta here:
5) This is what you have for the image code:
This is what you should have (note the subtle spaces + dashes instead of the colon like character in some places + also make PNG lowercase):
6) Brackets in these three places are unnecessary - the automatic referencing gives you references like (6) and (7), so no need for additional bracketing.
7) The most important bit now: In every major equation that you have, there is no space after the colon there and so the HTML doesn't give you auto-labelling. Make sure in all 10 of your major equations, there is that single space after the colon.
@thedukeofeelam have addressed all points, let me know if anything else is wrong!
@thedukeofeelam have addressed all points, let me know if anything else is wrong!
Thanks Noah! Will take a look first thing in the morning, and hopefully we can have these both merged before the end of today. Goodnighty.
1) The first equation doesn't get recognised, because again the space between the colon and L3_1 is missing in this particular equation 2) fix the alignment issue here. Just tab those certain lines to the right by 1 level
3) Add a space after "(18)"
4) While you are at it, just increase the width from 50% to maybe like 75 or 80%. Careful here, don't forget the spaces, otherwise we will have to do another commit haha.
Otherwise, everything is perfect in this lecture. @nozzington
@thedukeofeelam hopefully this is finally done
@thedukeofeelam hopefully this is finally done
Doing a little prayer 😂
Im away from my laptop, but yup make those commits for the two lectures, will review in the morning and update. Thanks Noah.
@nozzington it looks like this branch is not upto date with my main
branch; see image below which says you are 34 commits behind the angadhn/SpacecraftDynamics:main
. So you will have to follow the instructions in Contributing.md, just to ensure your fork is updated.
@nozzington Some minor changes/corrections to make from my review. A full sign-off will be from @thedukeofeelam.
[x] The square brackets could do with a slight increase in size and I think there are some stray $ signs. I forget the command but maybe it is \big
or something. Check with GPT.
[x] This next part is not rendering correctly in the text: The fix is to just have all of the items correlating to point 1 in the same cell. Otherwise I think html conversion gets messed up as the indentation levels are different.
[x] Maybe use a lighter shade of blue in the text below the image as the dark blue doesn't render nicely in dark mode. Or just go with default text for that one line of text about the semi-major axis.
[x] Remove the two explicit equations in point 4 and with the word Equations. The way you have set it up is great because the texts for (5) and (6) are in blue, meaning they are hyper links to the equation for the reader. We can save them trouble from reading repeated equations. You have done this perfectly here:
@nozzington also, please update author list to have Ilan's name after yours. Cheers.
@nozzington Yup, I only picked up on this: Just put all of the content into 1 cell, it worked when I tried.
For the brackets, GPT it, prompt something like: i give you equation, make brackets look nicer with left and right commands. Don't use the bigr and Bigr commands - it just doesn't work.
@angadhn , @thedukeofeelam all points should have been addressed pls let me know!
@angadhn , @thedukeofeelam all points should have been addressed pls let me know!
I am checking now :)
Lecture 3 and images, vectors fixed and made into bold face. hopefully this works, anything wrong I will fix.