Skip to content

docs: reorganize function reference#662

Merged
ntamas merged 58 commits into
igraph:mainfrom
maelle:reference-reorg
Mar 13, 2023
Merged

docs: reorganize function reference#662
ntamas merged 58 commits into
igraph:mainfrom
maelle:reference-reorg

Conversation

@maelle

@maelle maelle commented Feb 6, 2023

Copy link
Copy Markdown
Contributor

Fix #659
Based on #660

Comment thread src/Makevars Outdated
Comment thread man/roxygen/meta.R Outdated
Comment thread man/roxygen/meta.R Outdated
@maelle

This comment was marked as resolved.

@maelle

This comment was marked as resolved.

@maelle

maelle commented Feb 6, 2023

Copy link
Copy Markdown
Contributor Author

I'm constantly writing family as "famility" now 😹

@szhorvat

szhorvat commented Feb 6, 2023

Copy link
Copy Markdown
Member

is assortativity a structural property?

Yes, we can call it that, and I do suggest placing it into that section.

Frankly, all properties are "structural properties" and that section should eventually be broken up, once we get there. Unfortunately, at the moment that section is used as a bit of a dumping ground.

@szhorvat

szhorvat commented Feb 6, 2023

Copy link
Copy Markdown
Member

If you have any questions about the various concepts, feel free to ask.

@szhorvat

szhorvat commented Feb 6, 2023

Copy link
Copy Markdown
Member

I think that the documentation of IGraph/M, the Mathematica interface of igraph, is structured a bit better (and a bit more granularly than the C library's docs). You might find it helpful as a guide. http://szhorvat.net/mathematica/IGDocumentation/ Apologies about the giant doc page ... I just don't have the time, and Mathematica's doc tools are not there yet, so I needed to do a lot from scratch.

@ntamas

ntamas commented Feb 16, 2023

Copy link
Copy Markdown
Member

There are some deprecated layout functions in the "Versions" section. These don't belong there.

I don't see them either; I guess this point is now obsolete.

I think consolidating the lifecycle workflow (using the lifecycle package?) would make sense.

Totally agree, but I think that is not in the scope of this PR; we could do it separately in another PR. If you think that it's easier to make the deprecated functions disappear using the lifecycle package, just leave them as is for now and we'll update the docs when we adopt the usage of the lifecycle package.

Does layout.grid.3d count as deprecated or not?

Yes, it's deprecated.

Should "Scan statistics" be a subsection of "Structural properties"? Thoughts?

Let's make it a subsection.

@szhorvat

Copy link
Copy Markdown
Member

console() can go under "Interactive", it's fixed now.

  • There are some deprecated layout functions in the "Versions" section. These don't belong there. I would actually suggest removing these completely from the docs, as people shouldn't be using them (@ntamas, are you okay with that?)

Which ones?

I guess these are not really under "Versions", they just come at the end and appear to be under "Versions"? See here:

https://maelle.github.io/rigraph/reference/index.html#versions

I think everything should be excluded, except:

  • graph_version()
  • upgrade_graph()
  • igraph_version()
  • igraph_test()
  • console() (move to "Inrteractive")
  • %>% (but do we need this in the igraph docs?)

If this section is renamed to something else, e.g. "System", then igraph_test() will fit here just fine. Otherwise it needs a new home.

@maelle

maelle commented Feb 20, 2023

Copy link
Copy Markdown
Contributor Author

I had make a mistake in the pkgdown config syntax so all functions I had listed under "Internal" still showed up, sorry about that.

Are there further tweaks to make?

@maelle

maelle commented Mar 6, 2023

Copy link
Copy Markdown
Contributor Author

@krlmlr apart from the conflicts, ok to merge this?

@szhorvat

szhorvat commented Mar 6, 2023

Copy link
Copy Markdown
Member

Can we rename "Stochastic constructors – Random graph models (games)" to simply "Stochastic constructors (random graph models)"? I know I suggested the current title, but I just realize that the new dot-free names no longer use the "game"-terminology. So there's no reason to put that into the title.

Looking forward to this getting merged.

@ntamas

ntamas commented Mar 6, 2023

Copy link
Copy Markdown
Member

@maelle This conflicts with the base branch - is there an easy way to resolve the conflicts so this can be merged?

@szhorvat

szhorvat commented Mar 6, 2023

Copy link
Copy Markdown
Member

I'm looking through this once more, and here are a few more comments. @maelle I could make some of these changes myself, but I didn't want to interfere with your PR. Let me know if you prefer that I commit to this PR directly in the future.

  • "Spectral Coarse Graining" heading -> "Spectral coarse graining" (consistent case)
  • arpack() and arpack_defaults should not be in "Centrality measures". These can have their own top-level section.
  • spectrum() also doesn't belong to "Centralirty measures". It's an odd one out, but it can go in top-level "Structural properties"
  • "Hierarchical random graph functions" -> "Hierarchical random graphs" (it's nice if most headings fit on a single line without wrapping). The original uses the same title too.
  • "Emebdding" -> "Spectral embedding" (useful to be specific here)
  • diameter() goes under "Paths"
  • is_chordal() and max_cardinality_search() are both in the wrong place. They could go in top-level structural properties, or their own subsection on "Chordal graphs"
  • is_dag() shouls not be under "Paths". It can be included both in "Structural properties" and "Graph cycles".
  • feedback_arc_set() and girth() should appear in "Graph cycles" as well, in addition to their current location.
  • I'm not sure what printr is. Is it supposed to be in the docs @ntamas ?

This is probably not a subject of this PR, but hub_score() and authority_score() should share a single documentation page. Future versions will have a single function to compute both of these at the same time, in a way that the result match.

This reorganization is of course a never-ending task ... But this new doc overview page makes it much easier to find and correct issues.

@szhorvat szhorvat removed their request for review March 6, 2023 20:01
@maelle

maelle commented Mar 7, 2023

Copy link
Copy Markdown
Contributor Author

@maelle This conflicts with the base branch - is there an easy way to resolve the conflicts so this can be merged?

@ntamas I don't think they are huge conflicts but the interface tells me that only those with write access can merge the PR?

@krlmlr

krlmlr commented Mar 7, 2023

Copy link
Copy Markdown
Contributor

I resolved the conflicts by merging main into this branch and rerunning devtools::document() .

@maelle: Would you like to do one more iteration here?

@codecov

codecov Bot commented Mar 7, 2023

Copy link
Copy Markdown

Codecov Report

Merging #662 (9942f1e) into main (8f8e65a) will decrease coverage by 0.17%.
The diff coverage is n/a.

❗ Current head 9942f1e differs from pull request most recent head 2d25e9f. Consider uploading reports for the commit 2d25e9f to get more accurate results

@@            Coverage Diff             @@
##             main     #662      +/-   ##
==========================================
- Coverage   53.81%   53.65%   -0.17%     
==========================================
  Files         355      356       +1     
  Lines       73313    73544     +231     
==========================================
+ Hits        39457    39458       +1     
- Misses      33856    34086     +230     
Impacted Files Coverage Δ
R/aaa-auto.R 78.81% <ø> (ø)
R/attributes.R 78.00% <ø> (ø)
R/centrality.R 74.75% <ø> (ø)
R/community.R 74.63% <ø> (ø)
R/decomposition.R 80.00% <ø> (ø)
R/embedding.R 100.00% <ø> (ø)
R/epi.R 91.13% <ø> (ø)
R/eulerian.R 100.00% <ø> (ø)
R/flow.R 79.13% <ø> (ø)
R/games.R 76.37% <ø> (ø)
... and 11 more

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@maelle

maelle commented Mar 13, 2023

Copy link
Copy Markdown
Contributor Author

I changed the default branch which was causing quite a few problems when trying to fix conflicts.

Rebasing didn't work as I expected (I'm stuck a conflict in cigraph/), so I'll try merging as indicated above.

Merge branch 'main' into reference-reorg

# Conflicts:
#	R/centrality.R
#	man/authority_score.Rd
@maelle

maelle commented Mar 13, 2023

Copy link
Copy Markdown
Contributor Author

@ntamas @szhorvat time to merge or time for a few last tweaks? 😸

Comment thread .github/workflows/pkgdown.yaml Outdated
maelle added 2 commits March 13, 2023 13:42
Merge branch 'main' into reference-reorg

# Conflicts:
#	_pkgdown.yml
#	man/dot-apply_modifiers.Rd
#	man/dot-extract_constructor_and_modifiers.Rd
@maelle maelle requested review from ntamas and szhorvat March 13, 2023 14:30
@ntamas ntamas merged commit 00705ce into igraph:main Mar 13, 2023
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 13, 2024
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.

Improve pkgdown configuration for reference index

4 participants