search

litebird / litebird_sim

Simulation tools for LiteBIRD
GNU General Public License v3.0
18 stars 13 forks source link

correct doc typos #231

Closed marcobortolami closed 1 year ago

marcobortolami commented 1 year ago

I went through the documentation in docs/source/ and found some typos, uncorrectly linked API functions, ... . I created this PR to fix them.

marcobortolami commented 1 year ago

Few additional comments:

  1. In my opinion, this picture that is used here should be clarified a little bit more, e.g. by using the notation in the text just above it and by adding other vectors.
  2. Some of the links in README.md are not working because the corresponding sections do not exist, e.g. Prerequisites or Installation. These sections should be created.
  3. The links to the API in the bulleted list here are still not working. Should they be changed, e.g., from :func:`.HwpSys.set_parameters` to :func:`.hwp_sys.HwpSys.set_parameters`?

Here a checklist to keep track of this:

marcobortolami commented 1 year ago

I tried to change a bit the picture of point 1 (by hand, I will improve it as soon as I understand better this point). I am certainly missing something, as the polarization angle in the figure should be 180°. Do you have any comment? polarization-direction

ziotom78 commented 1 year ago

Thanks! I did some work on this PR to fix the issues you mentioned:

  1. In my opinion, this picture that is used here should be clarified a little bit more, e.g. by using the notation in the text just above it and by adding other vectors.

Right, the new image should be clearer:

image

The new text clarifies that the vector e_x has been drawn twice because the one at the top represents the polarization direction in the detector's reference frame, and it's what gets rotated to p.

2. Some of the links in [README.md](https://github.com/litebird/litebird_sim/blob/aca5c5e373b17085f1720752afe65140c8899966/README.md) are not working because the corresponding sections do not exist, e.g. Prerequisites or Installation. These sections should be created.

Right, they were generated automatically for some ancient version of the file, but they were never updated. Fixed.

3. The links to the API in the bulleted list [here](https://litebird-sim.readthedocs.io/en/master/external/hwp_sys.html) are still not working. Should they be changed, e.g., from `` :func:`.HwpSys.set_parameters`  `` to `` :func:`.hwp_sys.HwpSys.set_parameters`  ``?

Yes, but that's not enough, as :func: should be changed to :meth:, as they are methods of the class HwpSys. Fixed.

If you agree with my changes, we can merge this PR.

marcobortolami commented 1 year ago

Thank you, Maurizio.

Right, the new image should be clearer:

Yes, it is! One last comment: the North direction is along a meridian, not a parallel, right? If so, I can change the word and close the PR.

Right, they were generated automatically for some ancient version of the file, but they were never updated. Fixed.

Thanks, I also removed the last broken "Acknowledgements" link.

Yes, but that's not enough, as :func: should be changed to :meth:, as they are methods of the class HwpSys. Fixed.

Thanks.