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

#373 Include documentation_helpers in module #395

Merged

Conversation

arjxn-py
Copy link
Contributor

This Pull Request aims to fix #373 by including documentation_helpers in the submodule ragna._docs.
It'll help us to -

  • Not to explicitly include extra files in the documentation that need to be included through sys.path, reducing the noise in the example
  • Help users to run tutorials if they have ragna installed
  • Have the advantage of not being bound to backward compatibility even after we adopt semantic versioning given that the module is private.

cc: @pmeier

@arjxn-py
Copy link
Contributor Author

arjxn-py commented Apr 26, 2024

I find it strange that after I moved documentation_helpers to ragna._docs, I'm getting this error -

Traceback (most recent call last):
  File "/mnt/d/Arjun/ragna/docs/tutorials/gallery_python_api.py", line 19, in <module>
    import ragna._docs.documentation_helpers as documentation_helpers
  File "/mnt/d/Arjun/ragna/ragna/_docs/documentation_helpers.py", line 12, in <module>
    from ragna.deploy import Config
  File "/mnt/d/Arjun/ragna/ragna/deploy/__init__.py", line 8, in <module>
    from ._config import Config
  File "/mnt/d/Arjun/ragna/ragna/deploy/_config.py", line 133, in <module>
    class Config(ConfigBase):
  File "/mnt/d/Arjun/ragna/ragna/deploy/_config.py", line 139, in Config
    default_factory=ragna.local_root
AttributeError: module 'ragna' has no attribute 'local_root'

Which to me seems to be unrelated to the changes at the moment & I've also tried to fix it and found that local_root has been properly imported in ragna/__init__.py hence it should be an attribute.

I'd be really glad to have your input on this, until then I'm on it 🙂

@arjxn-py
Copy link
Contributor Author

I find it strange that after I moved documentation_helpers to ragna._docs, I'm getting this error

It has been fixed since now I'm importing local_path directly from ragna._utils. Thanks

docs/tutorials/gallery_python_api.py Outdated Show resolved Hide resolved
docs/tutorials/gallery_python_api.py Outdated Show resolved Hide resolved
Copy link
Member

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Thanks @arjxn-py. I left a few comments.

docs/tutorials/gallery_python_api.py Outdated Show resolved Hide resolved
ragna/_docs/documentation_helpers.py Outdated Show resolved Hide resolved
docs/examples/gallery_streaming.py Outdated Show resolved Hide resolved
ragna/deploy/_config.py Outdated Show resolved Hide resolved
@arjxn-py arjxn-py requested a review from pmeier April 26, 2024 23:32
@arjxn-py
Copy link
Contributor Author

Hi @pmeier, sorry to ping you. Just wanted to have a bit of your input on this while moving forward. Thanks.

arjxn-py and others added 6 commits April 30, 2024 17:05
…ve old sample text file

The changes include:

- Definition of `SAMPLE_CONTENT` in `ragna._docs`
- Creation of `ragna.txt` document when running tutorials
- Add new `ragna.txt` to .gitignore
Copy link
Member

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Thanks Arjun for the PR! I took the liberty and did the final minor cleanup to avoid lengthy back and forth.

@pmeier pmeier merged commit 04168ab into Quansight:main May 1, 2024
10 checks passed
@arjxn-py
Copy link
Contributor Author

arjxn-py commented May 1, 2024

Thanks Arjun for the PR! I took the liberty and did the final minor cleanup to avoid lengthy back and forth.

Thanks, I'm really grateful.
I'd be really happy to have your feedback on if I made any mistakes while working around in this PR & something I can improve upon to not repeat in the future.

@arjxn-py arjxn-py deleted the #373-include-documentation_helpers-in-module branch May 1, 2024 07:38
@pmeier
Copy link
Member

pmeier commented May 1, 2024

I'd be really happy to have your feedback on if I made any mistakes while working around in this PR & something I can improve upon to not repeat in the future.

The PR was good as is, but I had a few nitpicks that I did not want to bother you with. If you have a look at the commits I have sent, I just moved stuff around in the galleries and fixed a few typing issues that were not caused by you, but rather were already present in the old module. They weren't discovered earlier, because we didn't check the docs code. But now, since we have it in our regular package, it is checked.

pierrotsmnrd added a commit that referenced this pull request May 13, 2024
commit 04168ab
Author: Arjun Verma <arjunverma.oc@gmail.com>
Date:   Wed May 1 12:35:41 2024 +0530

    #373 Include documentation_helpers in module (#395)

    Co-authored-by: Philip Meier <github.pmeier@posteo.de>

commit f947f2d
Author: Philip Meier <github.pmeier@posteo.de>
Date:   Mon Apr 29 17:08:40 2024 +0200

    bump mypy to 1.10 (#398)

commit e0fe014
Author: Kevin Klein <7267523+kklein@users.noreply.github.com>
Date:   Mon Apr 29 12:52:49 2024 +0200

    Add instructions to install from conda-forge. (#396)

    Co-authored-by: Philip Meier <github.pmeier@posteo.de>

commit 7963453
Author: Philip Meier <github.pmeier@posteo.de>
Date:   Thu Apr 25 20:38:50 2024 +0200

    use custom JSON type for database for more generic support (#389)

commit 3a5b82d
Author: William Black <125844868+smokestacklightnin@users.noreply.github.com>
Date:   Wed Apr 24 01:37:48 2024 -0700

    Remove Mosaic Assistant (#387)
pmeier added a commit that referenced this pull request Jun 28, 2024
Co-authored-by: Philip Meier <github.pmeier@posteo.de>
pmeier added a commit that referenced this pull request Jun 28, 2024
Co-authored-by: Philip Meier <github.pmeier@posteo.de>
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.

ModuleNotFoundError: No module named 'documentation_helpers'
2 participants