Extended YAML

[cgranade]

Third time is the charm? This PR continues the work of #163, targeting develop, and with commits affecting APT devices moved onto another branch. For some reason, though, pylint seems to have gotten significantly more aggressive. I think it's related to the 13 April release, which adds a whole mess of new checks. I'll start a new branch to fix those, with the intent of merging that into this PR to try and fix the new lint issues raised by pylint 1.7.

[scasagrande]

Oh ffs pylint. Ok, I'll do what I should have done a while ago, which is fix the version of pylint in dev-requirements.txt. A green build, when rerun, shouldn't magically fail.

[cgranade]

Agreed. That said, the new checks in 1.7 seem to so far be suggesting reasonable changes anyway, so maybe it would make sense to fix for that upgrade then lock?

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling 29ffc57 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling 29ffc57 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling 29ffc57 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[scasagrande]

I don't know why we're getting an import error for py3.5 for ruamel. The unit tests pass fine locally, and I can import the package in an ipython env, but pylint fails. :/

[cgranade]

Side question, but for the purpose of diagnosing pylint issues, is it OK for me to hide information-level (I:) warnings? It's hard to find the actual issue amongst all the I: messages telling us we locally-disabled checks where they don't make sense.

[cgranade]

This is flummoxing, I can't seem to reproduce the pylint error locally. What might make sense is to locally disable the check until we can figure it out; if it ever actually fails to import, then the unit tests should catch that.

[scasagrande]

Yes feel free to disable the ignore messages

[scasagrande]

I can reproduce it, but only with Python 3.5 :/

[cgranade]

That is weird as all hell. I'll investigate a bit more, then. Doing so, it seems that since Travis is building off of 12.04, which predates 3.5, they install 3.5 in a different way than for the other builds. My guess would be that somehow pylint calls the system pylint instead of the one installed for 3.5, such that not all external dependencies are there.

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling ccda2a1 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling 8711b96 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling 8711b96 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling 8711b96 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling f73a243 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[cgranade]

f73a243 didn't work, so I'll revert that one, but short of that, I think we've done due diligence such that we can locally disable the import-error check and address this more thoroughly in #166.

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling 8d96981 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[coveralls]

Coverage increased (+0.3%) to 84.931% when pulling 5b148b6 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[scasagrande]

da fuck? really? What version is that?

[cgranade]

Anaconda, at least, seems to rename it to avoid a conflict somewhere. Ran into that trying to locally reproduce the pylint failure.

[scasagrande]

Well that's a thing.

[coveralls]

Coverage increased (+0.3%) to 84.931% when pulling 3099d07 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[coveralls]

Coverage increased (+0.3%) to 84.947% when pulling afca981 on feature-extended-yaml-no-apt into bc4fdbb on develop.

[cgranade]

I think this should cap it off, with 3099d07 and afca981 now following the suggestion made by ruamel.yaml 0.13.0 and marking calls to load as explicitly unsafe. This is unfortunately required by the fact that we need to be able to load arbitrary instrument classes, such that any YAML file for use with IK must be explicitly trusted. That said, we already clearly warn about that in the documentation, so I think it makes sense to follow ruamel.yaml's suggestion and mark unsafe loads.

(Side note: since nosetests is currently warning-free, perhaps at some point we should ratchet up code standards by making it fail on warnings instead of just errors?)

[scasagrande]

ugly, but yeah can help with debugging

[scasagrande]

good call, should help a lot. I wonder if there's a way to put that in the .pylintrc file to make it the default. I'm good with this though

[scasagrande]

Well that's a thing.