Skip to content

Conversation

@OsHCuellar
Copy link
Contributor

Updates to improve the generation of the documentation

  • Added a file to generate rst files automatically without importing dependencies.
  • Added a README.md in docs with the instructions to create the documentation.
  • Update the html theme.
  • Fix bugs with logo.
  • Update the documentation static files.

@OsHCuellar OsHCuellar requested a review from briling October 31, 2025 15:50
@briling briling mentioned this pull request Oct 31, 2025
@liam-o-marsh
Copy link
Contributor

I just added something to document the command-line uses
the one issue is that it requires the docs to import the files that are command-line invocable, so it does need the full set of dependencies.
Is this acceptable?

@briling
Copy link
Contributor

briling commented Nov 10, 2025

I just added something to document the command-line uses the one issue is that it requires the docs to import the files that are command-line invocable, so it does need the full set of dependencies. Is this acceptable?

I build them in the same env for now, but not sure how the pipeline is going to work. @OsHCuellar @romaingrx ?

Alternatively we can move parsers (and the defaults and exported names for methods) to separate files with minimal dependencies, if it's convenient

@liam-o-marsh
Copy link
Contributor

Actually, let me check something.
If it works, we would only need to minimise the imports on regression/parser.py

@romaingrx
Copy link

I just added something to document the command-line uses the one issue is that it requires the docs to import the files that are command-line invocable, so it does need the full set of dependencies. Is this acceptable?

I build them in the same env for now, but not sure how the pipeline is going to work. @OsHCuellar @romaingrx ?

Alternatively we can move parsers (and the defaults and exported names for methods) to separate files with minimal dependencies, if it's convenient

I believe we can setup a dedicated group (already the case with docs-build), build the html from the makefile and simply push the public folder to GitHub pages with this. Happy to commit the GitHub action if needed, just let me know.

@briling
Copy link
Contributor

briling commented Nov 11, 2025

I finally figured out how to automatically deploy docs:

https://github.com/briling/pages-test/blob/master/.github/workflows/dynamic.yml

This action rebuilds the "documentation" when i push to master and uploads is here https://briling.github.io/pages-test/ without commiting documentation rst/html anywhere.

I personally prefer this solution as the cleaner one. What you guys @OsHCuellar @romaingrx @liam-o-marsh think? Are there any drawbacks I don't see including security related?

@romaingrx
Copy link

That’s the way to go in my opinion, simply build the HTML in the GitHub action worker and push that to GitHub pages.

@briling
Copy link
Contributor

briling commented Nov 11, 2025

ah, it's not push in git terms, i see

@liam-o-marsh
Copy link
Contributor

...not sure why my previous commit was rebased but oh well.

the new commit adds a toggle in generate_rst.py (on by default) that "imports" the parser-generating functions in a different way, without importing the whole file they are in.

currently it still needs the sklearn dependency because the argparse-generators in qstack/regression import qstack.regression.parser, and that import statement implicitly pulls qstack.regression.__init__ in.

@briling
Copy link
Contributor

briling commented Nov 11, 2025

can we make qstack.regression.__init__ not raise if it is imported in this specific way?

@briling
Copy link
Contributor

briling commented Nov 11, 2025

@liam-o-marsh it also wants q-stack to be installed

@briling
Copy link
Contributor

briling commented Nov 11, 2025

and needs the C stuff to be built

(docs) 18:36:11-xe@dhcp-173-dist-b-094:~/GIT/Q-stack/docs> python generate_rst.py ../qstack/ -o source/ --project "Qstack" --package-root-name qstack
Traceback (most recent call last):
  File "/home/xe/GIT/Q-stack/docs/generate_rst.py", line 332, in <module>
    raise SystemExit(main())
                     ^^^^^^
  File "/home/xe/GIT/Q-stack/docs/generate_rst.py", line 313, in main
    mi = extract_module_info(py, mod_name)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/xe/GIT/Q-stack/docs/generate_rst.py", line 174, in extract_module_info
    parser = gg['_get_arg_parser']()
             ^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 2, in _get_arg_parser
  File "/home/xe/GIT/Q-stack/docs/../qstack/regression/__init__.py", line 13, in <module>
    from . import kernel_utils
  File "/home/xe/GIT/Q-stack/docs/../qstack/regression/kernel_utils.py", line 6, in <module>
    from .local_kernels import local_kernels_dict
  File "/home/xe/GIT/Q-stack/docs/../qstack/regression/local_kernels.py", line 118, in <module>
    'G_custom_c' : custom_C_kernels('G'),
                   ^^^^^^^^^^^^^^^^^^^^^
  File "/home/xe/GIT/Q-stack/docs/../qstack/regression/local_kernels.py", line 50, in custom_C_kernels
    dist_func = manh.eucl
                ^^^^^^^^^
  File "/home/xe/SOFT/miniconda3/envs/docs/lib/python3.12/ctypes/__init__.py", line 392, in __getattr__
    func = self.__getitem__(name)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/home/xe/SOFT/miniconda3/envs/docs/lib/python3.12/ctypes/__init__.py", line 397, in __getitem__
    func = self._FuncPtr((name_or_ordinal, self))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: /home/xe/GIT/Q-stack/docs/../qstack/regression/lib/manh.so: undefined symbol: eucl

@briling
Copy link
Contributor

briling commented Nov 11, 2025

ah okay it didn't like the old .so, now it works but still needs scikit-learn, numpy, and tqdm (!). missing qstack can be solved with sys.path.append

@briling
Copy link
Contributor

briling commented Nov 11, 2025

I think the solution is to set the kernels/models/etc names in one place and assign values later. I'll see what can be done

@briling
Copy link
Contributor

briling commented Nov 11, 2025

I think I know how to do it, but with code modification it will be a pain to merge with #115 #116 #117

@briling
Copy link
Contributor

briling commented Nov 11, 2025

@liam-o-marsh I fixed it for regression but we also have main functions in qstack/spahm/rho... I am not sure it is worth it

@briling
Copy link
Contributor

briling commented Nov 11, 2025

@OsHCuellar is it possible to make them look like a tree on the left bar? maybe even collapsable

also if we're doing documentation for CLI tools it would be nice to have them also somewhere separately

@liam-o-marsh
Copy link
Contributor

I think I'm happy about the state of things
Sometimes the module-level docstrings don't appear in the docs, but that's because they are misplaced, so this should work again once we also have the updated docs working

Do you think this is ready to be merged, or not yet?

@briling briling merged commit d9ceba0 into master Nov 17, 2025
6 of 7 checks passed
@briling briling deleted the automatic_docs branch November 17, 2025 12:47
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.

5 participants