Skip to content

docs: Improve min_st_separators() documentation#1264

Merged
aviator-app[bot] merged 5 commits intomainfrom
docs/min_st_separators
Mar 5, 2024
Merged

docs: Improve min_st_separators() documentation#1264
aviator-app[bot] merged 5 commits intomainfrom
docs/min_st_separators

Conversation

@szhorvat
Copy link
Copy Markdown
Member

@maelle Is this the best way to comment on examples? Can we comment using normal text that includes math instead of writing code comments? If yes, can you push to this branch and demonstrate?

@szhorvat szhorvat requested a review from maelle February 25, 2024 11:54
@aviator-app
Copy link
Copy Markdown
Contributor

aviator-app bot commented Feb 25, 2024

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.


See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.

@maelle
Copy link
Copy Markdown
Contributor

maelle commented Feb 26, 2024

Good question!

We can't have maths in example but a workaround would be to create an arbitrary section using R Markdown support. The code in the code chunk is evaluated every time the document() function is run,

  • which seems fine as far as time is concerned as this is fast code apparently;
  • which made me add the same code as a test case.

https://roxygen2.r-lib.org/articles/rd-formatting.html#code-chunks

Note that the section name is currently really bad. 😁

@szhorvat
Copy link
Copy Markdown
Member Author

Thank you!

@szhorvat
Copy link
Copy Markdown
Member Author

A problem is that every time we re-generate docs, something changes:

image

Should we do something about this?

Perhaps we should just go with the original code comment version? The math is actually not that important for this specific example.

@krlmlr krlmlr changed the title docs: improve min_st_separators() documentation docs: Improve min_st_separators() documentation Mar 2, 2024
@krlmlr
Copy link
Copy Markdown
Contributor

krlmlr commented Mar 2, 2024

In the tests, we have local_igraph_options(print.id = FALSE) to inhibit this, but I'm not sure what's the best way to use this with a roxygen2 example.

@maelle: Can you help please?

@krlmlr krlmlr marked this pull request as draft March 2, 2024 15:19
@maelle
Copy link
Copy Markdown
Contributor

maelle commented Mar 4, 2024

Probably not the best solution as this is an unexported function 🤪 (so especially confusing for users) I need to make it work to put the call in a separate chunk (using the .in argument), will try again later this week.

@szhorvat
Copy link
Copy Markdown
Member Author

szhorvat commented Mar 4, 2024

Let's just add the comments as an actual code comment, problem solved. Agreed?

I would rather not add distracting code to an example which is making an important point about how a function behaves.

@maelle
Copy link
Copy Markdown
Contributor

maelle commented Mar 5, 2024

I'll experiment with a knitr hook to modify the output without a comment. I fear the comment is distracting too.

@maelle maelle marked this pull request as ready for review March 5, 2024 06:39
@maelle maelle force-pushed the docs/min_st_separators branch from de5b7d5 to 2b37c19 Compare March 5, 2024 06:39
@maelle maelle removed their request for review March 5, 2024 06:39
@aviator-app aviator-app bot merged commit 7b99c05 into main Mar 5, 2024
@aviator-app aviator-app bot deleted the docs/min_st_separators branch March 5, 2024 07:09
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants