search

cabouman / mbircone

BSD 3-Clause "New" or "Revised" License
11 stars 9 forks source link

readthedoc fix #92

Closed dyang37 closed 2 years ago

dyang37 commented 2 years ago

Readthedocs failed to build online because scipy library is recently introduced to the code based, but not included in docs/requirements.txt. Scipy is used for 3D interpolation in multires.

dyang37 commented 2 years ago

@DamonLee5 Wenrui, could you please help me test the online build if you have time? Thanks!

gbuzzard commented 2 years ago

How do we know when docs/requirements.txt needs to include a package or not? There are some packages in the base requirements.txt that don't appear in the one in the docs folder, but this one seems to be required.

Would it be more robust to include the base requirements.txt in docs/requirements.txt and then docs/requirements.txt can also include anything specific for sphinx?

In general, I don't like having to maintain redundant information.

Greg

From: Diyu Yang @.> Sent: Friday, August 26, 2022 12:32 PM To: cabouman/mbircone @.> Cc: Buzzard, Gregery T @.>; Review requested @.> Subject: [cabouman/mbircone] readthedoc fix (PR #92)

---- External Email: Use caution with attachments, links, or sharing data ----

Readthedocs failed to build online because scipy library is recently introduced to the code based, but not included in docs/requirements.txt. Scipy is used for 3D interpolation in multires.

You can view, comment on, or merge this pull request online at:

https://github.com/cabouman/mbircone/pull/92

Commit Summary

File Changes

(2 fileshttps://github.com/cabouman/mbircone/pull/92/files)

Patch Links:

— Reply to this email directly, view it on GitHubhttps://github.com/cabouman/mbircone/pull/92, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AM4YSVDHYPU66LPD4PK6CZDV3DWS7ANCNFSM57XKFIIQ. You are receiving this because your review was requested.Message ID: @.***>

DamonLee5 commented 2 years ago

With the .readthedoc.yaml file added before my internship, currently, we don't need to maintain redundant packages in two requirements.txt

I tried to clean up the duplicate packages and tested them in my readthedoc from the forked mbircone. Here is my fork modification.

Here is the result webpage from forked mbircone and the log information from the readthedoc.

dyang37 commented 2 years ago

Removed redundant package requirements in docs/requirements.txt as suggested by Wenrui.

DamonLee5 commented 2 years ago

Looks good. I think it is ready to merge.