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

Add draft vignette #26

Merged
merged 18 commits into from
Dec 16, 2024
Merged

Add draft vignette #26

merged 18 commits into from
Dec 16, 2024

Conversation

cforgaci
Copy link
Contributor

@cforgaci cforgaci commented Nov 21, 2024

@fnattino, I drafted the vignette. I suggest, we call the package data bucharest and include streets and river_centreline in it. For now, I disabled the code chunks, but once the data are in, we can re-enable them. That will also produce the plots in the rendered articles.

@cforgaci cforgaci requested a review from fnattino November 21, 2024 13:53
@cforgaci cforgaci self-assigned this Nov 21, 2024
@fnattino
Copy link
Contributor

Nice idea! Also pinging @vanlankveldthijs who is working on the packaged data as part of #2 . It would then be nice to include also the river_centerline to the packaged data (in an analogous way as this is done in the CRiSp package).

Copy link
Contributor

@fnattino fnattino left a comment

Choose a reason for hiding this comment

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

Looks great @cforgaci! Indeed, when #27 is merged, we can finalize this.

vignettes/using-rcoins.Rmd Show resolved Hide resolved
vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
@fnattino fnattino marked this pull request as ready for review December 16, 2024 12:34
Copy link
Contributor

@fnattino fnattino left a comment

Choose a reason for hiding this comment

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

Hey @cforgaci , some other comments on the vignette - we can merge only after #31 as we depend on that bug fix in order to run the from_edge section.

vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
vignettes/using-rcoins.Rmd Show resolved Hide resolved
vignettes/using-rcoins.Rmd Show resolved Hide resolved
vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
vignettes/using-rcoins.Rmd Show resolved Hide resolved
vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
vignettes/using-rcoins.Rmd Show resolved Hide resolved
vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
Co-authored-by: Francesco Nattino <[email protected]>
vignettes/using-rcoins.Rmd Outdated Show resolved Hide resolved
@cforgaci cforgaci merged commit 8dbfdd9 into main Dec 16, 2024
4 checks passed
@cforgaci cforgaci deleted the 14-vignette-cf branch December 16, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants