Skip to content

docs: fix sample_degseq() example#1297

Merged
aviator-app[bot] merged 2 commits intomainfrom
fix-1289
Mar 22, 2024
Merged

docs: fix sample_degseq() example#1297
aviator-app[bot] merged 2 commits intomainfrom
fix-1289

Conversation

@maelle
Copy link
Copy Markdown
Contributor

@maelle maelle commented Mar 12, 2024

fix #1289

  • renamed variables, added explaining variables for clarity.
  • conditionally use withr, to make sure it works.

note that rlang::check_installed() will prompt the user to install withr, which is handy IMHO.

@aviator-app
Copy link
Copy Markdown
Contributor

aviator-app bot commented Mar 12, 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 maelle changed the title docs: rename variables for clarity docs: fix sample_degseq() example Mar 12, 2024
@maelle maelle requested review from krlmlr and szhorvat March 12, 2024 11:09
@maelle maelle marked this pull request as ready for review March 12, 2024 11:09
@krlmlr krlmlr changed the title docs: fix sample_degseq() example docs: fix sample_degseq() example Mar 12, 2024
@szhorvat
Copy link
Copy Markdown
Member

szhorvat commented Mar 12, 2024

How about just doing something like the following and avoiding withr?

while (! is_graphical(degs <- sample(1:100, 100, replace = TRUE, prob = (1:100)^-2)) ) {}

This keeps sampling until we hit on a graphical degree sequence (which happens with a high probability in this case).

I am not really in favour of the name vl_graph ... The Viger-Latapy method creates a simple and connected undirected graph, but we probably don't want to use a variable that spells all that out. I'd rather stick to g1, g2, g3, etc. and spell out their properties later.

No other comments except that I really hate the current method names and I am hoping that #876 will be fixed soon ...

@maelle maelle marked this pull request as draft March 19, 2024 09:27
@maelle maelle force-pushed the fix-1289 branch 2 times, most recently from 3448508 to 37ca70b Compare March 22, 2024 13:47
@maelle maelle marked this pull request as ready for review March 22, 2024 13:48
@aviator-app aviator-app bot merged commit c21378c into main Mar 22, 2024
@aviator-app aviator-app bot deleted the fix-1289 branch March 22, 2024 14:19
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 23, 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.

error

2 participants