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

python/tuple not accepted by intake-0.5.1's yaml.safe_load() in examples/dashboard.yml #766

Closed
jfx319 opened this issue Jul 17, 2019 · 2 comments

Comments

@jfx319
Copy link

jfx319 commented Jul 17, 2019

Oh, it's definitely YAML's fault, or at least JSON's fault (from which YAML inherits its scope). But the fact remains that it's awkward for Python users to write code like the above, so we should probably accept a list as well as a tuple unless we are already using the list/tuple distinction for some semantic purpose in this case.

Originally posted by @jbednar in #676

@jfx319
Copy link
Author

jfx319 commented Jul 17, 2019

There was an mar 18, 2019 commit to intake which replaces yaml.load() with yaml.safe_load() that seems to break the example dashboard from reading python/tuple

intake.open_catalog('dashboard.yml')
ConstructorError: could not determine a constructor for the tag 'tag:yaml.org,2002:python/tuple'
  in "<unicode string>", line 13, column 15:
            xlim: !!python/tuple [-8240227, -8231284]
                  ^

This stackoverflow answer seems to suggest that safe_load() can't handle python/tuple which is how I found the intake commit that changed the loading behavior.

Downgrading to
conda install -c conda-forge intake=0.4.3 (which predates the yaml.safeload commit)
temporarily fixes this issue, but doesn't address the root cause of

  • whether intake has to use safe_load
  • and if so, implications for the dashboard.yml syntax

alternatively, reverting intake/utils.py to:

def yaml_load(stream):
    """Parse YAML in a context where duplicate keys raise exception"""
    with no_duplicate_yaml():
        return yaml.load(stream)

also fixes the issue (not a good fix, but just proves where the yaml syntax is being called), and furthermore, causes a deprecation warning message about the not specifying yaml.load(input, Loader=)

YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsafe. Please read https://msg.pyyaml.org/load for full details.
  return yaml.load(stream)

From their description there, using yaml.full_load() (a shortcut for yaml.load(input, Loader=yaml.FullLoader)) seems to work fine for me on intake=0.5.1, and it removes the deprecation warning as well. But this would be for the intake team to decide... if this is non-negotiable from intake, then the datashader/example/dashboard.yml would need to be updated for yaml.safe_load() to work, but it seems more restrictive in what it can accept.

In the mean time, new users would probably appreciate a working example dashboard one way or another (maybe downgrading the intake version? though I don't know if anything else depends on a newer intake version).

@jbednar
Copy link
Member

jbednar commented Jul 17, 2019

Fixed in datashader and hvplot master, but requires a release of both packages before the end-user experience is fixed.

@jbednar jbednar closed this as completed Jul 17, 2019
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

No branches or pull requests

2 participants