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

Analysis API #116

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

Analysis API #116

wants to merge 10 commits into from

Conversation

jgoizueta
Copy link
Contributor

@jgoizueta jgoizueta commented May 14, 2019

This provides an interface to the in-the-works Analysis API

  • Low level API (rest client with JSON responses)
  • Higher level API (Job/Schedule/Execution) objects
  • Low level API tests
  • Error handling (use specific exceptions)
  • Higher level API tests
  • Docs (guide, examples)
  • API ref (code comments)

@@ -0,0 +1,52 @@
from carto.analysis import AnalysisClient

def test_analysis(api_key_auth_client_usr):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1


ANSI_ESCAPE = re.compile(r'\x1B\[[0-?]*[ -/]*[@-~]')

def uncolored(text):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

attrib = ''.join([Color.COLORS[a] for a in attrib])
return attrib + str(text) + Color.END

ANSI_ESCAPE = re.compile(r'\x1B\[[0-?]*[ -/]*[@-~]')

Choose a reason for hiding this comment

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

expected 2 blank lines after class or function definition, found 1

def enable_colors(mode):
Color.enabled = mode

def colored(attrib, text):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@@ -0,0 +1,39 @@
import re

class Color:

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

]


def update(self):

Choose a reason for hiding this comment

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

too many blank lines (2)

@staticmethod
def print(*args):
print(*args)
@staticmethod

Choose a reason for hiding this comment

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

expected 1 blank line, found 0


TASK_GROUP = 'analysis' # we have always one task group named analysis

class DefaultOutput:

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

)
res = self.auth_client.send(url, 'GET', headers=HEADERS)
logs = self.auth_client.get_response_data(res, False)
if not logs is None:

Choose a reason for hiding this comment

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

test for object identity should be 'is not'

'accept': 'application/json'
}

class AnalysisClient(object):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@jgoizueta
Copy link
Contributor Author

The higher level API is not Python 2 compatible at the moment

@jgoizueta
Copy link
Contributor Author

For consistency with other APIs I think it'll be better to rename AnalysisContext as AnalysisClient, (which should be the most useful interface) and rename AnalysisClient to AnalysisApiClient or so (and perhaps make it private).

if self.summary is None:
return ''
if self.type == 'now':
return 'C:{c} S:{s} F:{f} L:{l} Q:{q} R:{r}'.format(
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for being an uninvited guest, but how about using a cartocolor palette instead?? :)

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