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

Create entry point for starfish/experiment/builder/cli.py #436

Merged
merged 1 commit into from
Aug 17, 2018

Conversation

ttung
Copy link
Collaborator

@ttung ttung commented Aug 16, 2018

  1. Create a "main" method for the script.
  2. Add an entry point for the pip package.

Test plan:

% pip install -e .
% build_sample_experiment --help

Connects to #418

@codecov-io
Copy link

codecov-io commented Aug 16, 2018

Codecov Report

Merging #436 into master will increase coverage by 0.65%.
The diff coverage is 94.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #436      +/-   ##
==========================================
+ Coverage   84.73%   85.39%   +0.65%     
==========================================
  Files          72       72              
  Lines        2765     2773       +8     
==========================================
+ Hits         2343     2368      +25     
+ Misses        422      405      -17
Impacted Files Coverage Δ
starfish/starfish.py 82.69% <100%> (+1.05%) ⬆️
starfish/experiment/builder/cli.py 78.57% <93.75%> (+78.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbc8a58...7231ed1. Read the comment docs.

setup.py Outdated
'console_scripts': "starfish=starfish.starfish:starfish"
'console_scripts': [
"starfish=starfish.starfish:starfish",
"build_sample_experiment=starfish.experiment.builder.cli:main",
Copy link
Member

Choose a reason for hiding this comment

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

I think b_s_e would surprise me if it showed up on $PATH after pip install starfish

Copy link
Member

Choose a reason for hiding this comment

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

Yep, I had similar thoughts. I think it might make more sense to include it as a sub-module of starfish. For now, tagging it with a name that associates with our package is enough for me because I want to get this out to collaborators.

Suggestion:
build_sample_starfish_experiment + new issue to include it in the starfish binary.

@ttung
Copy link
Collaborator Author

ttung commented Aug 16, 2018

Made this a subcommand of the starfish cli. Example invocation: starfish build --fov-count 2 --hybridization-dimensions '{"c": 2, "h": 2, "z": 3}' /tmp/foo

@ttung ttung force-pushed the tonytung-build-experiment branch 2 times, most recently from 41c6be9 to db9f6f1 Compare August 16, 2018 20:51
@ambrosejcarr
Copy link
Member

Lol, love that as a side effect of this change, it also displays the starfish ascii art.

@ttung
Copy link
Collaborator Author

ttung commented Aug 17, 2018

Lol, love that as a side effect of this change, it also displays the starfish ascii art.

The most important change. That the tool is wired up is the side effect. :)

1. Create a "main" method for the script.
2. Add an entry point for the pip package.

Test plan:
```
% pip install -e .
% build_sample_experiment --help
```
@ttung ttung changed the base branch from tonytung-move to master August 17, 2018 20:13
@dganguli
Copy link
Collaborator

dganguli commented Aug 17, 2018 via email

@ttung ttung merged commit 389bbe3 into master Aug 17, 2018
@ttung ttung deleted the tonytung-build-experiment branch August 17, 2018 20:28
@ttung ttung restored the tonytung-build-experiment branch August 17, 2018 21:28
@ttung ttung deleted the tonytung-build-experiment branch August 24, 2018 17:57
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.

None yet

5 participants