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

[DOC]Restructuring Contributing Doc #8921

Closed
iskode opened this issue Aug 2, 2021 · 2 comments
Closed

[DOC]Restructuring Contributing Doc #8921

iskode opened this issue Aug 2, 2021 · 2 comments
Assignees
Labels
doc Documentation Python Affects Python cuDF API.

Comments

@iskode
Copy link
Contributor

iskode commented Aug 2, 2021

Report needed documentation

Report needed documentation

The Contributing to cuDF documentation flow is somehow confusing for me and there are duplicate instructions. As a new comer, the place where it is stated Done! You are ready to develop for the cuDF OSS project. ( above Debug cuDF), lets me wandering a lot because there are many preceding instructions that are indicated as "alternatively" or "optional". Thus, I propose to move/add some elements to improve the flow and make instructions clearer for new comers like me.

Describe the documentation you'd like
The structure I would like to see is :

  • General dependency requirements
    OS, python, cuda, nvidia driver versions
  • Set the environment
    clone repo + create env
  • Build and test cuDF from source
    • Common builds (Mandatory for any contributor) and tests (if any)
    • C++ contributors
      specific Dependency requirements, Build instructions and Tests (DBT)
    • Python contributors
      • cudf
        DBT
      • cudf/dask-cudf
        DBT
      • cudf-kafka
        DBT
    • Java contributors
      DBT

Done! You are ready to develop for the cuDF OSS project.

  • Debugging cuDF
  • Automated Build in Docker Container
  • Code formatting
    • test flake8, black, isort, clang-format
    • pre-commit hooks

Steps taken to search for needed documentation
List any steps you have taken:

The current structure is

  • Setting Up Your Build Environment
    • Code Formatting
    • Get libcudf Dependencies
  • Script to build cuDF from source
    • Build from Source(1)
  • Debugging cuDF
  • Automated Build in Docker Container

Duplication in code formatting section:
Packages flake8 sort black clang-format are already stated in environment files. Thus installation instructions in this section is are duplicated as they will be installed during builds.
This is one reason I put this section at the end (retaining only tests) after builds/installations in my proposed plan. The other one is that installation has a high priority compared to code formatting. Anyway the bullet point Code! Make sure to update unit tests! in your first issue will point to this new place. I also propose to change this bullet point into Code! Make sure to add and run unit tests as well as check code format!

Now, let's expand (1):

  • Build from Source:
    • Clone the repository and submodules
    • Create the conda development environment cudf_dev
    • Build and install libcudf after its dependencies. CMake depends on the nvcc executable being on your path or defined in $CUDACXX
    • As a convenience, a build.sh script is provided in $CUDF_HOME. To execute the same build commands above, run the script as shown below. Note that the libraries will be installed to the location set in $INSTALL_PREFIX if set (i.e. export INSTALL_PREFIX=/install/path), otherwise to $CONDA_PREFIX.
    • To build only the C++ component with the script
    • To run tests (Optional):
    • Build the cudf python package, in the python/cudf folder
    • Like the libcudf build step above, build.sh can also be used to build the cudf python package, as shown below:
      Additionally to build the dask-cudf python package, in the python/dask_cudf folder:
    • The build.sh script can also be used to build the dask-cudf python package, as shown below:
    • To run Python tests (Optional):
    • Other build.sh options:

Here, I'm very confused. I want to contribute to python cudf code only so I must install library locally in edit mode. But looking at this section, I wonder:
Do I need to build C++ libcudf to contribute to python ?
The same goes for java, cudf-dask, etc...
I might suppose yes and try-see-adjust but I think this should be clearly stated! That's why I put sections for each contributor profile, in my proposal, to know exactly related dependencies, build instructions and tests to make sure it went well.

Finally, after following the current instructions, I've tried to modify the python code locally but it doesn't take any effect meaning my local build/installation is not editable. Did I miss something ?
Revising the instructions, I haven't seen any pip install -e which is the only way I know that allows to install a package in edit mode with python.

I can submit a PR with my suggestions but I don't know about common and specific dependencies for each contributor profile so I might let these parts empty as well as making the library editable.

@iskode iskode added Needs Triage Need team to review and classify doc Documentation labels Aug 2, 2021
@shwina shwina added Python Affects Python cuDF API. and removed Needs Triage Need team to review and classify labels Aug 3, 2021
@shwina
Copy link
Contributor

shwina commented Aug 3, 2021

I think this would be an extremely valuable contribution.

Do I need to build C++ libcudf to contribute to python ?

Yes, and I think we should make that explicit in our instructions.

Also agree about making editable installs.

@iskode
Copy link
Contributor Author

iskode commented Aug 3, 2021

Thank you for your reply.

rapids-bot bot pushed a commit that referenced this issue Aug 26, 2021
This PR is related to the raised issue [here](#8921). It is a refactoring of the contributing doc with instructions to locally build `cuDF` in edit mode.

Authors:
  - Ismaël Koné (https://github.com/iskode)

Approvers:
  - Ashwin Srinath (https://github.com/shwina)
  - Robert Maynard (https://github.com/robertmaynard)

URL: #9026
@iskode iskode closed this as completed Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Documentation Python Affects Python cuDF API.
Projects
None yet
Development

No branches or pull requests

2 participants