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: fix sample_degseq() example #1297

Merged
merged 2 commits into from Mar 22, 2024
Merged

docs: fix sample_degseq() example #1297

merged 2 commits into from Mar 22, 2024

Conversation

maelle
Copy link
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.

Copy link
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
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
14 checks passed
@aviator-app aviator-app bot deleted the fix-1289 branch March 22, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

error
2 participants