Skip to content

Conversation

@adamnsch
Copy link
Contributor

@adamnsch adamnsch commented May 29, 2023

Thank you for your contribution to the Graph Data Science Client project.

Before submitting this PR, please read Contributing to the Neo4j Ecosystem.

Make sure:

  • You signed the Neo4j CLA (Contributor License Agreement) so that we are allowed to ship your code in our library
  • Your contribution is covered by tests

@netlify
Copy link

netlify bot commented May 29, 2023

Deploy Preview for neo4j-graph-data-science-client canceled.

Name Link
🔨 Latest commit 034f2bc
🔍 Latest deploy log https://app.netlify.com/sites/neo4j-graph-data-science-client/deploys/6475b3aee5931e0008cd96a4

@adamnsch adamnsch force-pushed the ref-docs-server-ep-coverage-test branch from 0c5a143 to 1c2b81b Compare May 29, 2023 09:16
@adamnsch adamnsch force-pushed the ref-docs-server-ep-coverage-test branch from 1c2b81b to 7cdb00b Compare May 29, 2023 11:27
@adamnsch adamnsch marked this pull request as ready for review May 29, 2023 12:09

.. py:function:: gds.kcore.mutate(G: Graph, **config: Any) -> "Series[Any]"
Computes the k-core values in a network
Copy link
Contributor Author

@adamnsch adamnsch May 29, 2023

Choose a reason for hiding this comment

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

This line is taken directly from the procedure description

:members:
:inherited-members:
:inherited-members:
:exclude-members: __init__
Copy link
Contributor Author

@adamnsch adamnsch May 29, 2023

Choose a reason for hiding this comment

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

The class itself already gets the __init__ params documentation, so if we don't exclude __init__ like this they are duplicated

Comment on lines +95 to +96
.. deprecated:: 2.4.0
Since GDS server version 2.4.0 you should use the endpoint :func:`gds.graph.sample.rwr` instead.
Copy link
Contributor Author

@adamnsch adamnsch May 29, 2023

Choose a reason for hiding this comment

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

I believe this is correct usage of this sphinx directive. But it does render kind of funny in our custom theme @nvitucci @recrwplay

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. the theme inherits the behaviour from the main docs. When a div has class="deprecated, we use the ::after pseudo-class on a <span> inside the <div> to show a deprecated label. We can modify the css in the Sphinx theme to override this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok :) That sounds good 👍 Is that something you want to do? Or do you need help from us?

The quotes are only needed in the actual Python code. Remove to make
more consistent with rest of the docs.
@adamnsch
Copy link
Contributor Author

The PR looks rather big, but that's just due to a harmless search and replace in the last commit

Copy link
Contributor

@breakanalysis breakanalysis left a comment

Choose a reason for hiding this comment

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

LGTM :)

Creates a node regression training pipeline in the pipeline catalog.

.. py:function:: gds.alpha.scaleProperties.mutate(G: Graph, **config: Any) -> Series[Any]
Copy link
Contributor

Choose a reason for hiding this comment

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

should these be marked with deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

Co-Authored-By: Jacob Sznajdman <breakanalysis@gmail.com>
@adamnsch adamnsch force-pushed the ref-docs-server-ep-coverage-test branch from f7a6064 to 034f2bc Compare May 30, 2023 08:28
@adamnsch adamnsch enabled auto-merge May 30, 2023 08:28
@adamnsch adamnsch disabled auto-merge May 30, 2023 12:33
@adamnsch adamnsch merged commit 20ac789 into neo4j:main May 30, 2023
@adamnsch adamnsch deleted the ref-docs-server-ep-coverage-test branch May 30, 2023 12:34
FlorentinD added a commit to FlorentinD/gdsclient that referenced this pull request Apr 18, 2024
Previously assumed it would be added twice without this exclusion (neo4j#383 (comment)).
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.

3 participants