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

LRE build error #2541

Merged
merged 10 commits into from
Oct 22, 2024
Merged

LRE build error #2541

merged 10 commits into from
Oct 22, 2024

Conversation

purva-thakre
Copy link
Contributor

Description

Test PR to check if the errors noticed by Nate persist for all builds. Tested the build locally without any issues.

image


License

  • I license this contribution under the terms of the GNU GPL, version 3 and grant Unitary Fund the right to provide additional permissions as described in section 7 of the GNU GPL, version 3.

Before opening the PR, please ensure you have completed the following where appropriate.

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.72%. Comparing base (8693d0e) to head (7d67e0e).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2541   +/-   ##
=======================================
  Coverage   98.72%   98.72%           
=======================================
  Files          90       92    +2     
  Lines        4158     4161    +3     
=======================================
+ Hits         4105     4108    +3     
  Misses         53       53           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@purva-thakre
Copy link
Contributor Author

purva-thakre commented Oct 17, 2024

IDK what the issue is. RTD build fails right after I push a new commit.

@purva-thakre
Copy link
Contributor Author

purva-thakre commented Oct 17, 2024

SO that might be possibly related to this issue.

It's not clear why the build passes locally but fails with a ModuleNotFoundError on RTD. \

Another related discussion in RTD repo on why a submodule couldn't be imported. Why only LRE import issues though? Why not other submodules?

Edit: I think this is happening because the submodules do not have init files.

@purva-thakre purva-thakre marked this pull request as ready for review October 17, 2024 17:45
@purva-thakre
Copy link
Contributor Author

purva-thakre commented Oct 17, 2024

@natestemen I managed to get the import issues fixed by adding init files in the submodules.
I am not too sure about what's preferred though because this deviates from what's followed in the submodules for other techniques.

https://app.readthedocs.org/projects/mitiq/builds/25982251/

@purva-thakre purva-thakre mentioned this pull request Oct 17, 2024
6 tasks
@purva-thakre
Copy link
Contributor Author

@natestemen this is ready to be merged.

@natestemen natestemen requested review from cosenal and removed request for natestemen October 22, 2024 16:57
Copy link
Contributor

@cosenal cosenal left a comment

Choose a reason for hiding this comment

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

nit: it's good practice for files to end with a new line (even those init files)

@cosenal
Copy link
Contributor

cosenal commented Oct 22, 2024

@natestemen I managed to get the import issues fixed by adding init files in the submodules. I am not too sure about what's preferred though because this deviates from what's followed in the submodules for other techniques.

https://app.readthedocs.org/projects/mitiq/builds/25982251/

Having init files in the submodules is a good practice.

@purva-thakre purva-thakre merged commit ecfa7dc into main Oct 22, 2024
16 of 18 checks passed
@purva-thakre purva-thakre deleted the lre_docs_failure branch October 22, 2024 20:49
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.

2 participants