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.

Sign up for GitHub

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

Jump to bottom

[ENH] Refactor C++ include directory #1491

Closed
BradReesWork opened this issue Mar 30, 2021 · 2 comments · Fixed by #1582
Closed

[ENH] Refactor C++ include directory #1491

BradReesWork opened this issue Mar 30, 2021 · 2 comments · Fixed by #1582
Milestone
21.06

Comments

@BradReesWork
Copy link
Member

To better match the rest of RAPIDS, the structure of include should be "cpp/include/cugraph"

create a new directory structure and move existing files
@BradReesWork BradReesWork added this to the 0.20 milestone Mar 30, 2021
@seunghwak
Copy link
Contributor

seunghwak commented Mar 30, 2021 •
edited
Loading

A related question is whether to include files under include/cugraph as

#include <file_name>
or
#include <cugraph/file_name>

It seems like the latter is common within RAPIDS, so we may follow that convention.

@seunghwak seunghwak changed the title [ENH] Refactor C include directory [ENH] Refactor C++ include directory Mar 30, 2021
@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@ChuckHastings ChuckHastings mentioned this issue May 7, 2021
Merged
rapids-bot bot pushed a commit that referenced this issue May 12, 2021
@trxcllnt
Move headers into include/cugraph dir (#1582)
4905a35 
This PR moves the headers in `cpp/include` into `cpp/include/cugraph` and updates C++ and Cython `#include` directives.

This change makes it easier for other libs to avoid naming conflicts with cuGraph public headers when linking `libcugraph.so`.

closes #1491

Authors:
  - Paul Taylor (https://github.com/trxcllnt)

Approvers:
  - Chuck Hastings (https://github.com/ChuckHastings)
  - Seunghwa Kang (https://github.com/seunghwak)
  - Rick Ratzel (https://github.com/rlratzel)

URL: #1582
@dantegd dantegd mentioned this issue May 27, 2021
Merged
rapids-bot bot pushed a commit that referenced this issue Jun 1, 2021
@dantegd
Correct install path for include folder to avoid double nesting (#1630)
e80a696 
Similar to rapidsai/cuml#3901. 

After #1491 and #rapids-cmake and #1585, now at install time, the cugraph headers are being nested into `path/to/env/include/cugraph/cugraph` instead of just `path/to/env/include/cugraph/`. This, as far as I'm aware, is unintentional and unlike the rest of RAPIDS projects (cuDF, RMM and cuML). 

cc @trxcllnt @robertmaynard

Authors:
  - Dante Gama Dessavre (https://github.com/dantegd)

Approvers:
  - Chuck Hastings (https://github.com/ChuckHastings)
  - Robert Maynard (https://github.com/robertmaynard)
  - Seunghwa Kang (https://github.com/seunghwak)
  - Paul Taylor (https://github.com/trxcllnt)
  - Rick Ratzel (https://github.com/rlratzel)

URL: #1630
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
21.06
Development

Successfully merging a pull request may close this issue.

Move headers into include/cugraph dir trxcllnt/cugraph
2 participants
@BradReesWork @seunghwak