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 dashboard_script.py #6

Merged
merged 1 commit into from May 26, 2018

Conversation

Projects
None yet
2 participants
@apoorvaeternity
Copy link
Member

commented May 20, 2018

This is the main script that will be run using the cron.
It creates a dataset_details.json file that stores the md5 and status of dataset.
I have used FileLock for updating the dataset_details.json using multiple threads.

The script is still under testing right now

@apoorvaeternity apoorvaeternity requested a review from henrykironde May 20, 2018

@@ -1,3 +1,4 @@
Django==2.0.2
Django==1.11
filelock

This comment has been minimized.

Copy link
@henrykironde

henrykironde May 20, 2018

Contributor

Why are we using 1.11?

This comment has been minimized.

Copy link
@apoorvaeternity

apoorvaeternity May 20, 2018

Author Member

Django from version 2.0 has stopped supporting python 2.
So for supporting both python 2 and 3 we will have to stick to Django version 1.11

This comment has been minimized.

Copy link
@henrykironde

henrykironde May 20, 2018

Contributor

We don't have to support python 2 for the dashboard project.

This comment has been minimized.

Copy link
@apoorvaeternity

apoorvaeternity May 21, 2018

Author Member

Okay 👍

@henrykironde

This comment has been minimized.

Copy link
Contributor

commented May 20, 2018

if we are using that style of docs strings, I think we should include all parameters.

def create_diff(csv1, csv2, diff_file, context, numlines):
    """
    Parameters
    ----------
    csv1 : The first csv file.
    csv2 : The second csv file.
    diff_file : The diff_file that is to be generated.
    context: .....
    numlines: .....
@@ -30,8 +31,8 @@ def check_dataset(dataset):
status = None
try:
md5 = get_dataset_md5(dataset)
if dataset.name not in dataset_detail or md5 != dataset_detail[
dataset.name]['md5']:
if (dataset.name not in dataset_detail

This comment has been minimized.

Copy link
@henrykironde

henrykironde May 22, 2018

Contributor

remove the () they are redundant.

from filelock import FileLock
from retriever import datasets

from status_dashboard_tools import (

This comment has been minimized.

Copy link
@henrykironde

henrykironde May 22, 2018

Contributor

I really do not feel comfortable with this .🙂 let's change it to one line.

This comment has been minimized.

Copy link
@apoorvaeternity

apoorvaeternity May 23, 2018

Author Member

Hehe. Okay 😄


file_location = os.path.dirname(os.path.realpath(__file__))

example_datasets = ['abalone-age', 'airports', 'biodiversity-response', 'bird-size',

This comment has been minimized.

Copy link
@henrykironde

henrykironde May 22, 2018

Contributor

This is too long, P8P recommends nothing longer than 80, unless you compromise readability.

example_datasets = [
    'abalone-age',
    'airports',
    'biodiversity-response',
    'bird-size',
    'breast-cancer-wi',
    'butterfly-population-network',
    'partners-in-flight',
    'portal']

or

example_datasets = [
    'abalone-age', 'airports',
    'biodiversity-response', 'bird-size',
    'breast-cancer-wi', 'butterfly-population-network',
    'partners-in-flight', 'portal']
@@ -55,6 +60,13 @@ def create_diff(csv1, csv2, diff_file, context, numlines):
csv1 : The first csv file.
csv2 : The second csv file.
diff_file : The diff_file that is to be generated.
context : set to True for contextual differences (defaults to False
which shows full differences).

This comment has been minimized.

Copy link
@henrykironde

henrykironde May 22, 2018

Contributor

I would reword this

def create_diff(csv1, csv2, diff_file, context, numlines):
    """
    Parameters
    ----------
    csv1 : The first csv file.
    csv2 : The second csv file.
    diff_file : The diff_file that is to be generated.
    context : Defaults to false for full diff, set to True for contextual diff.

This comment has been minimized.

Copy link
@henrykironde

henrykironde May 22, 2018

Contributor

I feel the second part needs to be made more clear.
numlines : Number of context lines before and after diff. Set to True to controls number of code lines before and after the change. Set to False ..... I am having hard time to comprehend what Set to False.. does

This comment has been minimized.

Copy link
@apoorvaeternity

apoorvaeternity May 23, 2018

Author Member

Setting to false shows the whole file. The changes and also those lines which do not have changes.

This comment has been minimized.

Copy link
@apoorvaeternity

apoorvaeternity May 23, 2018

Author Member

And when False is set for context the numlines are used for positioning the next anchor tag before the change. If context is set to false and numlines is set to 2 then it will place the anchor tag 2 lines before the change.

@apoorvaeternity apoorvaeternity changed the title [WIP] Add dashboard_script.py Add dashboard_script.py May 23, 2018

@apoorvaeternity apoorvaeternity force-pushed the apoorvaeternity:dashboard_script branch from df336ae to bb852f1 May 23, 2018

@henrykironde

This comment has been minimized.

Copy link
Contributor

commented May 26, 2018

Use an ignore or example list as in https://github.com/weecology/retriever/blob/master/retriever/try_install_all.py#L77.
I recommend that you start with a small set of datasets that can actually run easily. The datasets like predicts, vertnets* take a long time. There some datasets like bioclim, Mammal Super Tree, breed-bird-survey* that cannot load into the databases on the current master branch.

If we can run the code with minimal datasets, that will be good.
Later, we can add datasets to the list or remove.

@apoorvaeternity apoorvaeternity force-pushed the apoorvaeternity:dashboard_script branch from 2da1f9c to cb38e14 May 26, 2018

@henrykironde henrykironde merged commit 86a89a3 into weecology:master May 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.