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

Reworked interface #5

Merged
merged 17 commits into from
May 11, 2020
Merged

Reworked interface #5

merged 17 commits into from
May 11, 2020

Conversation

nevrome
Copy link
Member

@nevrome nevrome commented May 8, 2020

Here's a suggestion for a better interface in accordance to #4.

./sidora.R -h
usage: sidora.cli [--] [--help] [--as_tsv] [--entity_type ENTITY_TYPE]
       [--entity_id ENTITY_ID] [--credentials CREDENTIALS] [--cache_dir
       CACHE_DIR] module

MPI-SHH Pandora DB command line interface test

positional arguments:
  module             One of: summary, list, view, projects, tags

flags:
  -h, --help         show this help message and exit
  -a, --as_tsv       For module: list - Return the list as
                     tab-separated data

optional arguments:
  -t, --entity_type  For module: summary, list, view - One of: project,
                     tag, site, individual
  -i, --entity_id    For module: summary, list, view - Identifier of
                     one or multiple: projects, tags, sites,
                     individuals
  -c, --credentials  path to the credentials file [default:
                     .credentials]
  -d, --cache_dir    path to table cache directory [default:
                     /tmp/sidora.cli_table_cache]
./sidora.R summary -t site -i "..."


|Site_Id |Name          |Country            |Tags                         |Projects    |
|:-------|:-------------|:------------------|:----------------------------|:-----------|
|...|...|...|...|...|
      +--------+---------+----------+---------+----------+----------+---------++
      |                                                                        |
      |                                                                        |
      |                                                                        |
   50 +                                                                        +
L     |                                   +                                    |
a     |                                                                        |
t     |                                                                        |
i     |                                                                        |
t   0 +                                                                        +
u     |                                                                        |
d     |                                                                        |
e     |                                                                        |
  -50 +                                                                        +
      |                                                                        |
      |                                                                        |
      |                                                                        |
      +--------+---------+----------+---------+----------+----------+---------++
              30        35         40        45         50         55        60
                                      Longitude
./sidora.R list -t site -i "...","..."


|Site_Id |Name           |Country            |Tags                         |Projects    |
|:-------|:--------------|:------------------|:----------------------------|:-----------|
|...|...|...|...|...|
|...|...|...|...|...|
./sidora.R view -t site -i "..."


|Site_Id |Name |Country |Tags |Projects |
|:-------|:----|:-------|:----|:--------|
There is no entity with that entity_id. Did you mean one of the following entities?
- ... 
- ...
- ... 
- ... 
- ... 

I think it's neat. Fight me! 😄

@nevrome
Copy link
Member Author

nevrome commented May 8, 2020

But of course: Everything at an early prototyping stage.

@nevrome nevrome closed this May 8, 2020
@nevrome nevrome reopened this May 8, 2020
@nevrome nevrome requested review from jfy133, stschiff and TCLamnidis May 8, 2020 16:19
@nevrome
Copy link
Member Author

nevrome commented May 9, 2020

./sidora.R summary -t site -i "..."


|Site_Id |Name           |Country |Tags          |Projects    |
|:-------|:--------------|:-------|:-------------|:-----------|
|...|...|...|...|...|

             . .   ...  ...........                   .       ...
.  ................. .  ..  .....          ......  ..... .......................
     .   ..........    ...               ... ..........................    .
            .............  .            ..X............................
            ............              ..   .. ..... .................
               .......                ..... ..  ...................
                ..   .              ............ ....  ..........
                    .               .............        .    ..   .
                       .....             ..........          ..  .
                      ..........           ......              .      ..
                       ........            ......                    . .
                        .......            .....  .              ........
                        .....               ...                   ........
                        ..
                       ..                              .

                                                   ..     ..............
       .................          ..........................................
................................................................................

I love it.

@stschiff
Copy link
Contributor

Amazing. Love it!!! Just a question about --as_tsv: Shouldn't that be a sub-option for the "list" subcommand (or module?) Or are you currently not working with subcommand-specific options? Perhaps that's making things too complicated, but it would be somewhat more logical to have command-specific options, in my view. Just thinking loud.

Copy link
Contributor

@stschiff stschiff left a comment

Choose a reason for hiding this comment

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

I'm not following every bit here. I'd like to review more carefully the command line option interface at some point, but I don't see myself doing that anytime soon, so I suggest we move on and I check it out, or discuss with you, at some later timepoints.

@nevrome
Copy link
Member Author

nevrome commented May 11, 2020

argparser does not support real subcommands. I did not use r-argparse, because it has some extra Python dependencies. So sub-options are not a thing here. I think that's not really a problem: Unused options are just ignored with the current setup.

I think I will merge this now and I have two additional things on my mind:

  1. I'm still not sure if this should just be an R package. All the view_, list_ and summary_* modules should probably be functions of a package. A package that is not supposed to be used in R, but only with a command line script that we could put into the inst directory.
  2. I suggest we use the next hackhour session to fill some of these modules with life. This is a task that can easily be split up and in an hour we can already have a very satisfying cli tool with some minimal views for all sorts of Pandora entities. I think Thiseas already started to work on the summary_sites thing last friday.

@TCLamnidis
Copy link
Member

Out of curiosity, how does the map react if you place a point in an area that does not get drawn by default (e.g. New Zealand)? Is that point added, or do you currently simply replace . with X?

@nevrome
Copy link
Member Author

nevrome commented May 11, 2020

Ah - a classic. I'm confident from the implementation that all and every sample point will be plotted no matter if it is on land or not. The code just splits the word into a grid and checks if there is a (or multiple) sample in a particular cell. If so, then we get an X.

@nevrome nevrome merged commit a0a6a3c into master May 11, 2020
@nevrome nevrome deleted the reworked_interface branch May 11, 2020 13:30
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.

3 participants