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 command line tools for roboflow-python #196

Merged
merged 20 commits into from
Nov 29, 2023
Merged

Add command line tools for roboflow-python #196

merged 20 commits into from
Nov 29, 2023

Conversation

tonylampada
Copy link
Collaborator

@tonylampada tonylampada commented Oct 19, 2023

Description

The main change introduce in this PR is the new roboflowpy command line tool to start making the api methods available from the command line.

CleanShot 2023-11-28 at 17 25 11

This PR also refactors some of the internal code moving towards a cleaner separation of concerns, moving the actual http calls to the rfapi.py adapter.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Refactoring

How has this change been tested, please provide a testcase or example of how you tested the change?

Tested on staging and production

Docs

  • Need to document the new features both here in the python package and the API, but it's non-blocking. I'll be working on both next.

@tonylampada tonylampada changed the title Upload tests2 Add command line tools for roboflow-python Nov 24, 2023
@tonylampada tonylampada marked this pull request as ready for review November 28, 2023 20:37
@tonylampada tonylampada requested review from capjamesg, ryanjball and EmilyGavrilenko and removed request for capjamesg November 28, 2023 20:37
@capjamesg
Copy link
Collaborator

capjamesg commented Nov 28, 2023

I am excited to review this, @tonylampada! Is this going to replace our JavaScript-based CLI?

@tonylampada
Copy link
Collaborator Author

I am excited to review this, @tonylampada! Is this going to replace our JavaScript-based CLI?

@capjamesg exactly! The idea is to expose as much functionality from the python package to the command line and eventually surpass and obsolete the current node cli!

@capjamesg
Copy link
Collaborator

@tonylampada How do I install this?

@tonylampada
Copy link
Collaborator Author

@tonylampada How do I install this?

@capjamesg pip install -e . from the root dir

@capjamesg
Copy link
Collaborator

I get the following error with the roboflow download command:

(base) james@jamesmac roboflow-python % roboflow download https://universe.roboflow.com/gdit/aerial-airport/dataset/1
Traceback (most recent call last):
  File "/Users/james/opt/anaconda3/bin/roboflow", line 8, in <module>
    sys.exit(main())
  File "/Users/james/Documents/GitHub/roboflow-python/roboflow/roboflowpy.py", line 181, in main
    args.func(args)
  File "/Users/james/Documents/GitHub/roboflow-python/roboflow/roboflowpy.py", line 14, in download
    w, p, v = args.datasetUrl.split("/")
ValueError: too many values to unpack (expected 3)

@tonylampada
Copy link
Collaborator Author

Sorry, lemme fix that

@tonylampada
Copy link
Collaborator Author

@capjamesg in the mean time you can probably do something like

roboflow download gdit/aerial-airport/1 -f voc

@capjamesg
Copy link
Collaborator

It would be nice to be able to pull from URLs, too. You can use an algorithm like:

  1. Check if a string is a URL.
  2. If the string is a URL, extract the path. Extract the org name, dataset name, and version.
  3. If the string is a org name, dataset name, and version, extract the information as you do now.

@tonylampada
Copy link
Collaborator Author

tonylampada commented Nov 29, 2023

@capjamesg

It's fixed, that command is going to work now :-)

@capjamesg
Copy link
Collaborator

It works! Thank you!

Copy link
Collaborator

@capjamesg capjamesg left a comment

Choose a reason for hiding this comment

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

One quick comment, then this looks good to go!



def _argparser():
parser = argparse.ArgumentParser(description="main description")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you set a full description for the package?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Woops, sorry about that. It's there now!

@tonylampada tonylampada merged commit 8864d37 into main Nov 29, 2023
3 checks passed
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.

2 participants