-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Config plot option #260
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
Config plot option #260
Conversation
from plotly import session | ||
|
||
|
||
class FileToolsTest(TestCase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\o\ |o| /o/
Otherwise, it’s pretty easy to munge them if tests change them!
About to add session stashing, so it’s helpful to specify which cre 8000 dentials/config we’re stashing/restoring.
@aneda you're tests are failing because you have an old version of master. You should handle it in this order:
Apologies if those instructions are overly detailed. I just want to make sure you're good to go! Also, let me know if you've got any questions 😸 |
@aneda what do you think about #260 (comment) I apologize for possibly steering you in a slightly wrong direction, but I'd like this to be as simple to implement as possible. Seeing the code so far, I'm realizing that the nested plot options might lead to more complication than it's worth. I'm really okay either way, as long as the file remains fairly stupid-proof. |
@@ -50,7 +50,7 @@ | |||
DEFAULT_PLOT_OPTIONS = { | |||
'filename': "plot from API", | |||
'fileopt': "new", | |||
'world_readable': True, | |||
'world_readable': False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@theengineear - My thought is to include saving the world_readable
as a "required" step in the new getting-started instructions and not changing the default behaviour. So, everyone who is new will save the default to False, and be aware of privacy, and we don't introduce the backwards incompatibility for everyone else who is upgrading.
thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, that sounds better to me! To be clear then, @aneda , let's change this back to 'world_readable': True
to maintain backwards compat.
@theengineear - yeah, changeling through github releases. still seems like the best way, since it includes the exact commit |
Config file safe tests
…-api into config_plot_option
…nfig_plot_option
``` ====================================================================== ERROR: Failure: TypeError (__init__() takes exactly 1 argument (2 given)) ---------------------------------------------------------------------- Traceback (most recent call last): File "/usr/local/lib/python2.7/site-packages/nose/loader.py", line 523, in makeTest return self._makeTest(obj, parent) File "/usr/local/lib/python2.7/site-packages/nose/loader.py", line 570, in _makeTest return self.loadTestsFromTestCase(obj) File "/usr/local/lib/python2.7/site-packages/nose/loader.py", line 494, in loadTestsFromTestCase return super(TestLoader, self).loadTestsFromTestCase(testCaseClass) File "/usr/local/Cellar/python/2.7.10/Frameworks/Python.framework/Versions/2. 7/lib/python2.7/unittest/loader.py", line 56, in loadTestsFromTestCase loaded_suite = self.suiteClass(map(testCaseClass, testCaseNames)) TypeError: __init__() takes exactly 1 argument (2 given) ```
@theengineear @chriddyp @cldougl |
8000
tr>
@aneda this is looking good. if you want me to pull this down and get tests running again let me know. they're failing because of a change @BRONSOLO recently made that we're being a bit too strict about in the api library, imo. That file needs to be updated to use a |
@theengineear did I do it right? |
test_file_tools pass in circle
|
||
Returns all if no arguments are specified. | ||
|
||
Example: | ||
get_credentials_file('username') | ||
get_credentials_file('plotly_domain') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_config_file
?
Looking good! One blocking comment about the _args to *_kwargs change. |
@theengineear is this good? Also would let me know if the issue does make sense or not, thanks! |
@theengineear just updated the kwargs to args |
hmmm, not sure why this new test is failing? i'll take a closer look tomorrow. @chriddyp do you think breaking a small amount of backwards compat is OK here: |
test_file_tools pass in circle
@theengineear I added another cc @chriddyp |
💃 ! |
If we're going to release on pypi, we also need a version bump! |
@theengineear @chriddyp is it good to go? |
si! 👯 |
merge away! On Mon, Jul 20, 2015 at 2:37 PM, Andrew notifications@github.com wrote:
Chris Parmer |
Config plot option: world_readable
@theengineear @chriddyp @cldougl
This is a draft for config_plot_option (world_readable). I haven't tested for python 3 yet but I wanted check if I am on the right track