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.

Sign up for GitHub

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

Jump to bottom

Upgrade YamlCpp to new 0.5 API #8

Merged
merged 22 commits into from
Jul 31, 2018
Merged

Upgrade YamlCpp to new 0.5 API #8

merged 22 commits into from
Jul 31, 2018

Conversation

breznak
Copy link
Member

@breznak breznak commented Jan 19, 2018 •
edited
Loading

YAML is a dependency, update to latest version, fix breaking changes as they moved to new API.

Fixes numenta#1319 #33
Moved from numenta#1320

Todo:

  • do the porting in Network.cpp, see the issue for details and help.
  • port YAMLUtils
  • resolve issue in CI (not reproducible locally) with linking Boost
    • by upgrading to YamlCpp 0.6 (c++11, no boost dependency)

@breznak
Copy link
Member Author

breznak commented Jan 19, 2018

https://ci.appveyor.com/project/numenta-ci/nupic-core/build/0.3.0.2255/job/tpqniaraoo0ctifo#L1172
a problem of the CMakeFile not setting up boost includes properly, or is it missing on the CI machines? (but it shouldn't matter, as we ship the boost headers ourselves).
I don't observe this on my system.
Likely with the update to YAML 0.5.7, we will be interested in the new release jbeder/yaml-cpp#415 that will not depend on boost. So workaround introduced now can then be removed.

@breznak breznak mentioned this pull request Jan 19, 2018
Closed
3 tasks
@breznak breznak modified the milestone: Community fixes Jan 23, 2018
@breznak
Copy link
Member Author

breznak commented Jan 24, 2018

@chhenning @mattheller if you are running Windows, do you think you could try having a look in to the problem?

@breznak breznak added the help wanted Extra attention is needed label Jan 24, 2018
@chhenning
Copy link

Good news is that your code changes are working on my Windows machine. Great work! I have internally upgraded to yaml-cpp 0.5.3.

Bad news is that I cannot really help with cmake. Sorry.

@breznak breznak closed this Jan 25, 2018
@breznak breznak reopened this Jan 25, 2018
breznak added 16 commits January 25, 2018 02:58
@breznak
upgrade yaml-cpp to 0.5.3 from old API 0.3.0
ee68efd
@breznak
update YAML to 0.1.7
e5f91ec
@breznak
YAML: port to LoadFile()
624d637 
replaces old API GetNextDocument()
@breznak
YAML: update API doc.FindValue() to doc["aa"]
f810a23 
as upgrading to YAML 0.5 from 0.3
@breznak
YAML: replace Iterator with for-each
7a1c7f8 
as upgrading to 0.5 API
@breznak
YAML Network: use node.as<int> for value retrieval
450d720 
replaces node >> invValue;
upgrade to 0.5 API
@breznak
YAML: convert YAMLUtils._toScalar
4b01f25 
use val = node.as<T>();
instead of node >> val;

upgrading to API 0.5 yaml
@breznak
YAMLUtils : remove _toScalar
697f209 
replaced with .as<T>()
@breznak
YAML Utils: remove toValue() from public API
fb59950
@breznak
Revert "YAMLUtils : remove _toScalar"
005169e 
This reverts commit c44f9c5.
@breznak
YAMLUtils: port _toArray to API 0.5
ecd6b3f
@breznak
YAMLUtils: port toValueMap() to 05. API
38979a3
@breznak
Revert "YAML Utils: remove toValue() from public API"
e9d1b10 
This reverts commit dac77df.
@breznak
YAML: use Load() instead of LoadFile for pure strings
6fbf8c5 
LoadFile is for filepath,
Load() is for yaml strings: '[1 2 3 4 5]'
@breznak
YAML: fix behavior on empty string
f6ac7b8 
proken during this PR, now fixed
@breznak
YAML: remove old workaround for 1-letter strings
f07e2d6 
apparently, yaml-cpp had a bug and did not handle 1 letter strings "1"
Now it seems to work ok.
Add test
@breznak breznak force-pushed the upgrade_yaml branch 2 times, most recently from fb4874d to 7da9a4b Compare January 25, 2018 03:05
breznak added 4 commits July 31, 2018 11:42
@breznak
YamlCpp: upgrade to 0.6.2
bf5c23e 
new API, many fixes since old release.
Removed dependency on Boost (causes problems in our CI machines)
@breznak
Revert "Boost: set include path in cmake"
07a8d86 
This reverts commit 7f05a3f.

As this causes errors in our CI, no longer required due to bump to
YamlCpp 0.6+
@breznak
YamlCpp update REadme
27fe12f
@breznak
YamlCpp: disable build tests to avoid error
07995e7 
as we were getting error with gtest, disabling tests for YamlCpp
workarounds the problem.
@breznak breznak added this to the Community fixes milestone Jul 31, 2018
@breznak breznak changed the title Upgrade yaml to new 0.5 API Upgrade YamlCpp to new 0.5 API Jul 31, 2018
@breznak breznak added dependencies backport_upstream What could be useful for upstream backporting labels Jul 31, 2018
@breznak breznak requested a review from rcrowder July 31, 2018 10:39
@breznak breznak self-assigned this Jul 31, 2018
@breznak breznak mentioned this pull request Jul 31, 2018
Closed
@breznak breznak added the enhancement New feature or request label Jul 31, 2018
@breznak
Copy link
Member Author

breznak commented Jul 31, 2018

@rhyolight should I tag you for changes like this that could be backported upstream to numenta, or you'll be watching out yourself?

@breznak breznak merged commit 1d5f41a into htm-community:master Jul 31, 2018
@breznak breznak deleted the upgrade_yaml branch July 31, 2018 10:48
@breznak breznak mentioned this pull request Jul 31, 2018
Closed
@breznak breznak mentioned this pull request Feb 26, 2019
Open
@ctrl-z-9000-times ctrl-z-9000-times mentioned this pull request May 24, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rcrowder rcrowder Awaiting requested review from rcrowder

Assignees

@breznak breznak

Labels
backport_upstream What could be useful for upstream backporting dependencies enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade to new YAML-cpp API (old v0.3.0 holds back yaml, yaml-cpp)
2 participants
@breznak @chhenning