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 graph_datasets to rosdistro #40803

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hridaybavle
Copy link
Contributor

@hridaybavle hridaybavle commented Apr 24, 2024

Package names:

Package Upstream Source:

https://github.com/snt-arg/graph_datasets.git

Purpose of using this:

Structured indoors synthetic dataset in the form of graphs as a tool for lidar_s_graphs.

PR related to lidar_s_graphs ---> #40800

Please Add This Package to be indexed in the rosdistro.

Iron and Humble

The source is here:

https://github.com/snt-arg/graph_datasets.git

Checks

  • [ x] All packages have a declared license in the package.xml
  • [ x] This repository has a LICENSE file
  • [ x] This package is expected to build on the submitted rosdistro

@github-actions github-actions bot added humble Issue/PR is for the ROS 2 Humble distribution iron Issue/PR is for the ROS 2 Iron distribution labels Apr 24, 2024
@hridaybavle hridaybavle changed the title Add graph datasets Add graph datasets to rosdistro Apr 24, 2024
@hridaybavle hridaybavle changed the title Add graph datasets to rosdistro Add graph_datasets to rosdistro Apr 24, 2024
@clalancette
Copy link
Contributor

New package review checklist

  • At least one of the following must be present
    • Top level license file:
    • Per package license files:
  • License is OSI-approved: GPLv3
  • License correctly listed in package.xmls
  • Public source repo: https://github.com/snt-arg/graph_datasets
  • Source repository contains ROS packages
  • Each package meets REP-144 naming conventions - Unfortunately this name is far too generic; this could be about many different concepts in robotics or even computer science. I'm going to suggest you rename this package to situational_graph_datasets, or snt_situational_graph_datasets.

Besides that, the source repository is missing a setup.cfg file, and the setup.py file has a bunch of TODOs in it.

@hridaybavle
Copy link
Contributor Author

Hello @clalancette, I have done the renaming of the repos as suggested by you previously. They now start with a prefix situational. Could you confirm this is okay so can I make commit to the other PRs as well. Thanks.

Also this source repo now has the setup.py and setup.cfg.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

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

New package review checklist

I'm going to approve this, but I will suggest that you update https://github.com/snt-arg/situational_graphs_datasets/blob/develop/setup.py to include the license and the maintainer.

@clalancette
Copy link
Contributor

Note that this needs to be rebased now because we merged some of the other packages. Once that is done, we can get this in, thanks!

@hridaybavle
Copy link
Contributor Author

Rebased with master :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
humble Issue/PR is for the ROS 2 Humble distribution iron Issue/PR is for the ROS 2 Iron distribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants