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

Sisl files not available in readthedocs documentation #745

Closed
pfebrer opened this issue Apr 11, 2024 · 18 comments · Fixed by #746
Closed

Sisl files not available in readthedocs documentation #745

pfebrer opened this issue Apr 11, 2024 · 18 comments · Fixed by #746

Comments

@pfebrer
Copy link
Contributor

pfebrer commented Apr 11, 2024

The SISL_FILES_TESTS variable is not set in readthedocs and it causes errors for some notebooks that plot files: https://sisl.readthedocs.io/en/latest/visualization/viz_module/index.html

This doesn't happen in the other documentation site:
https://zerothi.github.io/sisl/visualization/viz_module/index.html

@zerothi
Copy link
Owner

zerothi commented Apr 11, 2024

Yes, I seem to recall I disabled it on rtd because of time-outs, RTD has a limitation on the amount of time it allowed to run. We can try and re-enable it to see if will actually work.

@pfebrer
Copy link
Contributor Author

pfebrer commented Apr 11, 2024

Maybe I can try to change the tutorials so that they don't use files. But I think plotting bands files is what most people would want to do so it is nice to have it in the tutorial.

@zerothi
Copy link
Owner

zerothi commented Apr 11, 2024

Lets first see if we can get it up and running

@zerothi
Copy link
Owner

zerothi commented Apr 11, 2024

Can you see the build log here: https://readthedocs.org/projects/sisl/builds/24039054/
I can't understand why this fails, same sphinx version locally, and here all works...

@pfebrer
Copy link
Contributor Author

pfebrer commented Apr 12, 2024

If I install sisl in development mode I can't import sisl_toolbox, but it seems like it installs sisl normally, right?

@zerothi
Copy link
Owner

zerothi commented Apr 12, 2024

Hmm, what are you doing? I don't have any issues? Are you doing an editable intsall?

@pfebrer
Copy link
Contributor Author

pfebrer commented Apr 12, 2024

Yes, I meant editable install

@pfebrer
Copy link
Contributor Author

pfebrer commented Apr 12, 2024

Sorry, I can import it, but running stoolbox --help gives:

Traceback (most recent call last):
  File "/home/pfebrer/miniconda3/envs/simulations/bin/stoolbox", line 5, in <module>
    from sisl_toolbox.cli import stoolbox_cli
  File "/home/pfebrer/webDevelopement/sislGUI/sisl/src/sisl_toolbox/cli/__init__.py", line 11, in <module>
    from ._cli import *
ModuleNotFoundError: No module named 'sisl_toolbox.cli._cli'

@pfebrer
Copy link
Contributor Author

pfebrer commented Apr 12, 2024

import sisl_toolbox.siesta.atom

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/pfebrer/webDevelopement/sislGUI/sisl/src/sisl_toolbox/siesta/atom/__init__.py", line 6, in <module>
    from ._atom import AtomInput
  File "/home/pfebrer/webDevelopement/sislGUI/sisl/src/sisl_toolbox/siesta/atom/_atom.py", line 847, in <module>
    from sisl_toolbox.cli import register_toolbox_cli
  File "/home/pfebrer/webDevelopement/sislGUI/sisl/src/sisl_toolbox/cli/__init__.py", line 11, in <module>
    from ._cli import *
ModuleNotFoundError: No module named 'sisl_toolbox.cli._cli'

@zerothi
Copy link
Owner

zerothi commented Apr 12, 2024

Ah, I see now, I had completely forgotton to add this one in a prior PR... 2 secs

@zerothi
Copy link
Owner

zerothi commented Apr 12, 2024

try now ;)

@pfebrer
Copy link
Contributor Author

pfebrer commented Apr 12, 2024

Yes, now it works, so maybe it is the same problem that RTD had when importing sisl_toolbox.siesta.atom ?

@zerothi
Copy link
Owner

zerothi commented Apr 12, 2024

I think so, it is running just now ;)

@zerothi zerothi mentioned this issue Apr 12, 2024
5 tasks
@zerothi
Copy link
Owner

zerothi commented Apr 12, 2024

On another related note:

This goes fine:
https://readthedocs.org/projects/sisl/builds/24047173/

Yet, this one doesn't:
https://github.com/zerothi/sisl/actions/runs/8662894836/job/23755808712

You can even see it on the RTD page: https://sisl.readthedocs.io/en/latest/visualization/viz_module/index.html
There you can see broken thumbnails, and for some it works, for others it doesn't?

I also sometimes see this locally, the plot object not having a show method, why is that?

@pfebrer
Copy link
Contributor Author

pfebrer commented Apr 12, 2024

Hmm ok this is because the plot is not computed yet. I don't know how to handle this, because if the plot is not computed this kind of methods will fail, but if the plot has been computed it will work.

I could call .get() on the plot when some attribute is requested, but I don't know if I should do it only when the plot hasn't been computed yet, or in all cases. If I do it in all cases this means that the plot will get updated if inputs have been updated (it would trigger a computation of the plot). I don't know if this would be fine or not 😅

@zerothi
Copy link
Owner

zerothi commented Apr 12, 2024

But how could the plot not be computed yet? I mean it is a function call ;)
And the cells are executed?

@pfebrer
Copy link
Contributor Author

pfebrer commented Apr 12, 2024

No, it is not a function call, it is a Node call 😅 And nodes are lazy by default :)

So the question then is if by calling .show() the user wants to see the latest updated plot or just the current state of the plot.

@zerothi
Copy link
Owner

zerothi commented Apr 13, 2024

Hmm, I see. But how was it then working previously? There you also used 'show' for the thumbnails, are the png even necessary for the thumbnails?

It was because of my clean-up of the thumbnails in the docs.

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 a pull request may close this issue.

2 participants