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

docs: restructuralize examples - move all of them into a dedicated dir #4815

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

pesekon2
Copy link
Contributor

@pesekon2 pesekon2 commented Dec 7, 2024

Create directory docs/examples and move the following directories inside:

  • docs/gui
  • docs/notebooks
  • docs/python
  • docs/raster
  • docs/vector

@pesekon2 pesekon2 requested a review from wenzeslaus December 7, 2024 20:13
@pesekon2 pesekon2 self-assigned this Dec 7, 2024
@github-actions github-actions bot added Python Related code is in Python C Related code is in C HTML Related code is in HTML translation Message translation related libraries docs notebook labels Dec 7, 2024
@echoix echoix requested a review from neteler December 7, 2024 21:32
@echoix
Copy link
Member

echoix commented Dec 7, 2024

I'm just including Markus as we are finishing off a big documentation transition, and thus to make sure it is covered correctly and accounted for in the transition (for work possibly already done or tested without this change)

@pesekon2
Copy link
Contributor Author

I'm just including Markus as we are finishing off a big documentation transition, and thus to make sure it is covered correctly and accounted for in the transition (for work possibly already done or tested without this change)

The commit in talk is the already-merged ab90c5e, right? I have merged main into the PR to be up-to-date and solved some minimal conflicts popping up.

@neteler
Copy link
Member

neteler commented Dec 19, 2024

The commit in talk is the already-merged ab90c5e, right?

That's correct.

I have merged main into the PR to be up-to-date and solved some minimal conflicts popping up.

Great, thanks for taking care of the conflicts.

@neteler
Copy link
Member

neteler commented Dec 19, 2024

There is still a linting issue:

MARKDOWN
  2024-12-19 09:43:49 [INFO]   Linting MARKDOWN items...
  Error: -19 09:43:52 [ERROR]   Found errors when linting MARKDOWN. Exit code: 1.
  2024-12-19 09:43:52 [INFO]   Stderr contents for MARKDOWN:
  ------
  /github/workspace/doc/examples/notebooks/README.md:8:81 MD013/line-length Line length [Expected: 80; Actual: 91]

@pesekon2
Copy link
Contributor Author

There is still a linting issue:

MARKDOWN
  2024-12-19 09:43:49 [INFO]   Linting MARKDOWN items...
  Error: -19 09:43:52 [ERROR]   Found errors when linting MARKDOWN. Exit code: 1.
  2024-12-19 09:43:52 [INFO]   Stderr contents for MARKDOWN:
  ------
  /github/workspace/doc/examples/notebooks/README.md:8:81 MD013/line-length Line length [Expected: 80; Actual: 91]

Yes. The only reason it pops up now is that the original file contained it and it was moved now (without edits). It is not introduced by this PR, it is just that the linter finally got to check the file. I guess it should therefore be fixed in a separate PR (this one should not modify files unless necessary) -> I'll prepare a chore one that should be merged before this one.

@echoix echoix added the needs rebase Rebase to or merge with the latest base branch is needed label Dec 19, 2024
@echoix
Copy link
Member

echoix commented Dec 19, 2024

The PR with the line length fix was merged, so there's a small conflict here to solve (copy the changed file again here)

@pesekon2
Copy link
Contributor Author

The PR with the line length fix was merged, so there's a small conflict here to solve (copy the changed file again here)

Merged into the PR. Markdown issues solved. But there are still issues with some Python code - some of the moved Python example modules have their description too long.

./doc/examples/python/m.distance.py:28:89: E501 line too long (103 > 88 characters)
# % description: If the projection is latitude-longitude, this distance is measured along the geodesic.
                                                                                        ^

Is there any way to handle this? I found many places like this throughout the repository but none of them seemed to have to deal with this so far. They are just ticking ticking ticking timebombs waiting for a new commit to activate them (all of them seemed to be modified before the linter was introduced).

@echoix
Copy link
Member

echoix commented Dec 20, 2024

The PR with the line length fix was merged, so there's a small conflict here to solve (copy the changed file again here)

Merged into the PR. Markdown issues solved. But there are still issues with some Python code - some of the moved Python example modules have their description too long.


./doc/examples/python/m.distance.py:28:89: E501 line too long (103 > 88 characters)

# % description: If the projection is latitude-longitude, this distance is measured along the geodesic.

                                                                                        ^

Is there any way to handle this? I found many places like this throughout the repository but none of them seemed to have to deal with this so far. They are just ticking ticking ticking timebombs waiting for a new commit to activate them (all of them seemed to be modified before the linter was introduced).

https://github.com/OSGeo/grass/blob/main/.flake8 ?

@echoix echoix removed the needs rebase Rebase to or merge with the latest base branch is needed label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C docs HTML Related code is in HTML libraries notebook Python Related code is in Python translation Message translation related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants