Skip to content

First crack at resources. #311

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

Closed
wants to merge 1 commit into from
Closed

First crack at resources. #311

wants to merge 1 commit into from

Conversation

theengineear
Copy link
Contributor

Something I've been thinking about for a little while now.

We basically allow users to interact with Figure and Grid objects, but these are really just part of general File objects.

This is sorta what I've got in mind right now (and this is weakly implemented already in this PR):

from plotly import resources

my_file = resources.FileResource(fid='theengineear:3409')
my_file.resource.filename = 'nah, it should be named this'
my_file.put()  # if we don't have an error, things are good!
my_file.permanent_delete()  # ERROR trash first!
my_file.trash()
my_file.permanent_delete()

This isn't really ready to be merged in, but I wanted to open up the discussion with some actual code.

@theengineear
Copy link
Contributor Author

@chriddyp @cldougl @etpinard @BRONSOLO thoughts?

I think a super-thin wrapper around our underlying api might be pretty nice!

It would nicely solve the following within the same framework:

"From the api..."

  • "how do i delete my plots?"
  • "how can i lookup a plot by it's filename"
  • "how can i change the privacy of a plot"
  • "how can i get a list of my plots"
  • "how can i move a file into a folder"

This would be totally orthogonal to the idea of Figure and Grid creation. These objects would be wrappers that make it easier to handle the more general files.

The other option is to somehow build all of this into Figure and Grid objects. But I'm starting to thing that we'd be straying too far from the actual implementation? I don't think the abstraction is all that helpful?

I'd love any thoughts on this one. There are a few issues in this repo that I think this would solve cleanly.

@theengineear
Copy link
Contributor Author

Also, this is a general api-wrapper question. It'd be nice to have some sort of uniform way to do things like this across our wrappers, yeah?

@etpinard
Copy link
Contributor

👍

@chriddyp
Copy link
Member

I think @doeg had some thoughts on this too (from the node perspective)

@doeg
Copy link

doeg commented Sep 30, 2015

Cool! Yeah, (re?)writing the Plotly API as an npm module is ticketed here: https://github.com/plotly/streambed/issues/3013

We haven't been thinking about it yet, but I'm definitely interested in this discussion. The thinner and more closely coupled the wrapper is to our actual API the better, in my opinion.

If anyone has any examples of some especially well implemented APIs, I'd love to see 'em. (I know Twitter's entire API is an antipattern of what not to do...)

@chriddyp
Copy link
Member

part of me feels like we should just document how to do this well with requests. Read through their quick-start, as he explores the github api: http://docs.python-requests.org/en/v1.0.1/user/quickstart/

maybe the best approach is to document how to do these common tasks (with requests), and then see if there is any annoying boiler plate code that we can simplify for people.

the nice thing about keeping it tightly coupled to the API is that we'll only need to update the api.plot.ly docs

@theengineear
Copy link
Contributor Author

@doeg yeah the npm module spec you've got going is right inline with what I want for the Python API.

@chriddyp totally see your point about not maintaining another hunk of code. just thought I'd bring this up though. as our api gets more consistent, implementing a thin wrapper around it gets easier. that said, it's still something to maintain.

@theengineear
Copy link
Contributor Author

Hey! I ended up finally doing this in #640. It ended up really helping to unify our response handling! Closing this in favor of the changes in #640. It's basically a super thin wrapper around requests.request for all our api endpoints.

@theengineear theengineear deleted the file-objects branch December 28, 2016 21:37
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.

4 participants