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

Added info for conda installation problems #51

Merged
merged 20 commits into from
Jan 5, 2023
Merged

Conversation

Chris-N-K
Copy link
Contributor

@Chris-N-K Chris-N-K commented Nov 18, 2022

Description

As in napari/napari#5345 at the napari main repository discussed I added a small info box for the case that the conda installation does not work propperly.
In this case mamba is a good alternative.

Type of change

  • Bug-fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

References

@psobolewskiPhD recommended mamba

How has this been tested?

  • example: the test suite for my feature covers cases x, y, and z
  • example: all tests pass with my change

Final checklist:

  • My PR is the minimum possible work for the desired functionality
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • If I included new strings, I have used trans. to make them localizable.
    For more information see our translations guide.

@jaimergp

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Nov 18, 2022
@Czaki Czaki added the instalation instalation instructions label Nov 18, 2022
docs/tutorials/fundamentals/installation.md Outdated Show resolved Hide resolved
docs/tutorials/fundamentals/installation.md Outdated Show resolved Hide resolved
docs/tutorials/fundamentals/installation.md Outdated Show resolved Hide resolved
@psobolewskiPhD
Copy link
Member

Further down, at line 143 Checking that it worked perhaps lets add a sentence to run:
napari --version and confirm that the proper version was installed (ideally we clearly state the current version that should be expected, see comment above). We've had a couple issues where someone ended up with napari 0.4.12 or 0.4.14 due to solver issues and didn't realize it.

@Chris-N-K
Copy link
Contributor Author

If you state the current version one has to update the doc every time. Or can we add a prompt with the current version tag?

@psobolewskiPhD
Copy link
Member

If you state the current version one has to update the doc every time. Or can we add a prompt with the current version tag?

Yeah, I posted about that above, but it's resolved now: #51 (comment)
So will add here:
@melissawm is there a tag for the current version of napari in the docs, so we don't need to correct a reference to the version every time, if we want to add something like that here?

@psobolewskiPhD
Copy link
Member

@ch-n You want to take a stab at versioning the version number, per @melissawm :

Yes - if you have this stable_version variable defined in a myst_substitutions dictionary in the conf.py file, you can print it anywhere in the docs using the following syntax:

{{ stable_version }}

See this commit for an example.

@Chris-N-K
Copy link
Contributor Author

Yes will take a look.

Chris-N-K and others added 2 commits November 26, 2022 11:02
Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
@Chris-N-K
Copy link
Contributor Author

Chris-N-K commented Nov 26, 2022

Guess I just don't find it but the best solution I could come up is to us the link to the latest version. This is stable and dynamic but will not display the correct version tag. @melissawm @psobolewskiPhD

@psobolewskiPhD
Copy link
Member

Hmm, I tried the myst_substitution provided by @melissawm
psobolewskiPhD#2
It doesn't look like you can substitute in code blocks and the version is not the napari version—at least on CI:
image

@Czaki
Copy link
Contributor

Czaki commented Nov 26, 2022

this version problem is a well-known problem of tox as tox is not compatible with setuptools_scm.

@psobolewskiPhD
Copy link
Member

this version problem is a well-known problem of tox as tox is not compatible with setuptools_scm.

Does that mean it's just messed up in PR builds, or does it mean it will be messed up if released?

@melissawm
Copy link
Member

melissawm commented Dec 1, 2022

Hmm, I tried the myst_substitution provided by @melissawm psobolewskiPhD#2 It doesn't look like you can substitute in code blocks and the version is not the napari version—at least on CI:

Indeed - code blocks are supposed to not be interpreted by the parser so that makes sense.

@Chris-N-K
Copy link
Contributor Author

That would leave us with the link to the latest version I guess.

@psobolewskiPhD
Copy link
Member

@ch-n I made a PR to your fork with some suggestions.
I added the verbiage from Jaime, from the comment:
#51 (comment)
Also I implemented the version substitution.
I got the version number to work by tweaking the build action.
@Czaki @melissawm How does this fix look for y'all? Do the other actions need the same fix?
Finally, I worked around the code block issue somewhat. It's not perfect. Here's what it looks like:
image
image

You can see the built docs in my fork PR:
psobolewskiPhD#3

@Czaki
Copy link
Contributor

Czaki commented Dec 3, 2022

I have added in Chris-N-K#1 (review) the solution to provide napari version

obraz

psobolewskiPhD and others added 3 commits December 3, 2022 20:00
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
@psobolewskiPhD
Copy link
Member

Awesome @Czaki super clever!! I've merged your changes into the PR to @ch-n PR.

@github-actions github-actions bot added the task label Dec 5, 2022
@Chris-N-K
Copy link
Contributor Author

All merged now, guess it should be ready for a final review.

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Dec 6, 2022

@Chris-N-K @Czaki I made a PR adding python_version to the substitutions:
Chris-N-K#2
To make it work, I needed to also add a substitution for python={python_version} and the code block of conda env instruction, using the clever strategy from above.
You can check out the built docs in my fork:
https://github.com/psobolewskiPhD/napari-docs/actions/runs/3624911741

I also fixed the listing of supported python versions from 3.7 ... to 3.8 ... because we don't support 3.7 anymore.

For me this is not a blocker, so I leave it up to @Czaki and @jaimergp for the final approval.

Copy link
Contributor

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

Just one nit. Can be merged afterwards!

docs/tutorials/fundamentals/installation.md Outdated Show resolved Hide resolved
Chris-N-K and others added 2 commits December 20, 2022 11:41
Co-authored-by: Jaime Rodríguez-Guerra <jaimergp@users.noreply.github.com>
@Chris-N-K
Copy link
Contributor Author

Just one nit. Can be merged afterwards!

Merged it and fixed a conflict with main.
Happy this can go live now :)

@jaimergp
Copy link
Contributor

I have added in ch-n#1 (review) the solution to provide napari version

obraz

This is unrelated but I noticed that the font (face and size) is different in the numbered list. Is that intentional or a CSS bug?

@psobolewskiPhD
Copy link
Member

I resolved the conflict if we want the python version set programmatically as suggested by @Czaki Chris-N-K#2
@jaimergp the formatting does look a bit different. I've triggered the CI—not sure why we hadn't before—so we can get the docs built with this branch and take another look. Otherwise, maybe @melissawm has some thoughts on the formatting.

@melissawm
Copy link
Member

obraz

This is unrelated but I noticed that the font (face and size) is different in the numbered list. Is that intentional or a CSS bug?

I believe this is the current napari-sphinx-theme formatting/design choice. cc @codemonkey800 ?

@psobolewskiPhD
Copy link
Member

@Czaki @Chris-N-K Did y'all have any input on the python version substitution:
Chris-N-K#2
There's another PR in-flight that also refers to python version (https://github.com/napari/docs/pull/72/files#), so I think it's a decent idea to get it in somewhere to standardize.

@Chris-N-K
Copy link
Contributor Author

@Czaki @Chris-N-K Did y'all have any input on the python version substitution:
Chris-N-K#2
There's another PR in-flight that also refers to python version (https://github.com/napari/docs/pull/72/files#), so I think it's a decent idea to get it in somewhere to standardize.

I think the current solution is absolutely reasonable. There might be a slight lag when a new version is unrolled and the docs repo might not have the newest version for a short amount of time, but that should be no real problem.
If it is noted in the guide for doc additions should be fine and easy to use.

Merry Christmas ;)

@DragaDoncila
Copy link
Contributor

This is awesome, thanks folks! Merging now 🎉

@DragaDoncila DragaDoncila merged commit 153467c into napari:main Jan 5, 2023
psobolewskiPhD added a commit that referenced this pull request Feb 13, 2023
…ubstitution (#99)

# Description

See napari/napari#5509 (comment)
The docs live on napari.org don't match PR action downloaded docs when
doing napari version substitution
(#51)

This PR updates the deploy action with `fetch-depth: 0` that was added
to the PR `build_docs` action. This should make the version metadata be
correct (see pypa/setuptools-scm#480)

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Fixes or improves existing content
- [ ] Adds new content page(s)
- [ ] Fixes or improves workflow, documentation build or deployment

# References
Adresses issue noted here
napari/napari#5509 (comment)

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality
psobolewskiPhD added a commit to napari/napari that referenced this pull request Feb 14, 2023
…stitutions (#5557)

# Description

This PR an an analog to napari/docs#99
The built docs built by this repo PR action have improper napari version
substitution (see napari/docs#51)

This PR fixes that based on the README of
https://github.com/actions/checkout
It updates the deploy action with `fetch-depth: 0`. You can see that
this was added to the napari/docs PR build_docs action and works
correctly. This makes the version metadata be correct (see
pypa/setuptools-scm#480)

## Type of change
- [x] Bug-fix (non-breaking change which fixes an issue)

# References
Motivated by napari/docs#51
Analog to napari/docs#99
@psobolewskiPhD psobolewskiPhD added this to the 0.4.18 milestone Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation instalation instalation instructions task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants