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

Upgrade to BaseX 9 #47

Merged
merged 6 commits into from
May 23, 2018
Merged

Upgrade to BaseX 9 #47

merged 6 commits into from
May 23, 2018

Conversation

pralitp
Copy link
Contributor

@pralitp pralitp commented Apr 28, 2018

Upgrade to BaseX 9 and switch to it's new xquery serialization format meant to deal with serializing/parsing large CSV datasets.

This may help with #44.

meant to deal with serializing/parsing large CSV datasets
@pralitp pralitp self-assigned this Apr 28, 2018
Copy link
Contributor Author

@pralitp pralitp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: we should update the model interface submodule as well at the very least. I'm not sure if we want to wait to merge until we merge the BaseX 9 changes into master on the modelinteface repo AND I don't know if we want to propose to do this in GCAM yet.

@codecov
Copy link

codecov bot commented Apr 28, 2018

Codecov Report

Merging #47 into master will not change coverage.
The diff coverage is 100%.

Impacted Files Coverage Δ
R/querymi.R 83.84% <100%> (ø) ⬆️

@rplzzz
Copy link
Contributor

rplzzz commented Apr 28, 2018

I guess the answer to that depends on whether the baseX-9 changes are backward compatible. Here's what we need to consider:

  1. After the merge, will we still be able to read old GCAM output files written with baseX-8?
  2. Once GCAM is updated to baseX-9, will the baseX-8 version of rgcam still be able to read its outputs?

If the answer to both of these is yes, then we should increment the patch level of rgcam and go ahead with the merge.

If the answer to 1 is yes, but 2 is no, then we can still go ahead with the merge, but we should increment the minor version of rgcam.

If the answer to 1 is no, then we should increment the major version of rgcam and wait to merge until the corresponding merge goes into GCAM.

@pralitp
Copy link
Contributor Author

pralitp commented Apr 30, 2018

It looks like the answer to 1 is yes and 2 is no.

It sounds like we would be ok with having rgcam proceed with BaseX 9 ahead of GCAM in which case I will proceed by pointing the modelinteface submodule the branch on that repo.

the modelinterface (as apposed to master since we are upgraded rgcam
to BaseX 9 before GCAM).  Note the ModelInterface.jar also got rebuild,
this time with the minimum Java version as 8 instead of 7 as this is the
minimum in BaseX 9.  I've update the DESCRIPTION to reflect this.  Also
note we are implicitly getting fixes from the ModelInterface to support
running with Java 9/10.
BaseX 8 versions of rgcam won't be able to read BaseX 9 format databases
fix when querying results that did not specify Units.
@pralitp pralitp merged commit 2fecb71 into master May 23, 2018
@pralitp pralitp deleted the basex-9 branch May 23, 2018 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants