Resolve deprecation warnings for v0.7

[halleysfifthinc]

According to the testsuite, this fixes all the deprecations.

The only things I'm unsure of is the change in v0.7 for reinterpret/reshape to return reinterpret/reshape types. Since the tests check type equality (and error if mismatched type) before checking for equality in the contents, this required me to add a collect (for most of the cases) around the reinterpret or reshape. This will now allocate when it did not before, and may affect the read/write performance, however I don't see a way around this (other than changing the types of the test data to make the tests pass with reinterpret/reshape types).

[halleysfifthinc]

Tests fail due to the issues with WinRPM and Homebrew..

[Evizero]

What is the state on this?

[halleysfifthinc]

My understanding is that the PR is ready to be merged, depending on how important passing CI is for whoever has merge permissions. Tests pass locally on my machine (Linux x64), but the last run of Travis failed on 1.0 (bad travis config for 1.0, no using Pkg), tests on 0.7 failed because CMakeWrapper failed to build, Blosc failed because of CMakeWrapper. HDF5 failed to install because the apt sources returned 404's; my guess is that is a Travis issue, and I have no clue if there is anything we can do to solve that on our end.

Obviously, once this gets merged, any issues with 1.0 can be fixed.

[c-p-murphy]

Thanks for doing this.

[musm]

just julia 0.7

[halleysfifthinc]

The single OSX failure on Travis is due to the gcc link failing in homebrew. Not sure what caused the Linux failure.

[halleysfifthinc]

All green! @simonster or @yuyichao could we get this merged now?

[aliddell]

All green! @simonster or @yuyichao could we get this merged now?

+1

[yuyichao]

Since the tests check type equality (and error if mismatched type) before checking for equality in the contents, this required me to add a collect (for most of the cases) around the reinterpret or reshape. This will now allocate when it did not before, and may affect the read/write performance, however I don't see a way around this (other than changing the types of the test data to make the tests pass with reinterpret/reshape types).

Most of those looks wrong. You should now allocate the array you want to return and fill the reinterpreted/reshaped version.

[halleysfifthinc]

Ok. I will see about changing that.

[yuyichao]

I believe the copying issue is only needed to maintain the output type which shouldn't break most of the use so merging PRs that doesn't include that part is certainly fine and I've done that already.

[oxinabox]

Just to be clear master is still broken in 1.0
because a ton of the basic deprecatation removing stuff has not been done (because it is done in this PR).

And master is also broken on 0.7 is also broken because of the reinterpretted output types thing.

[halleysfifthinc]

I noticed the same thing about the updates to master; it was unusable for me (as of a week ago). I haven't had the time recently to take a look at this again, but the brief look I took at the PR to HDF5 linked earlier in this thread looked to be on the right track. I may have some time in the next week or two to take another stab at this.

On Fri, Nov 16, 2018, 10:30 AM Lyndon White ***@***.***> wrote: Just to be clear *master* is still broken in 1.0 because a ton of the basic deprecatation removing stuff has not been done (because it is done in this PR). And *master* is also broken on 0.7 is also broken because of the reinterpretted output types thing. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#86 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHA_LfV4pYyWJaf6515Cq70slOMckWqVks5uvtoygaJpZM4VnKUN> .

-- -AH

[nicoleepp]

The HDF5 changes needed to fix the extra copies here haven't been tagged yet:
JuliaIO/HDF5.jl#535

[oxinabox]

Here is the METADATA PR.
It is currently kinda stalled
JuliaLang/METADATA.jl#19579

[oxinabox]

The HDF5 changes are merged and tagged. Thanks @musm

[halleysfifthinc]

I think that should fix all the extra copies. If somebody (@yuyichao perhaps) can review to double-check, then this will be good to go once JuliaIO/HDF5.jl#538 is merged and tagged.

[yuyichao]

See JuliaIO/HDF5.jl#538 (comment)

[davidssmith]

Can we get this merged?

Do you know what is worse than a performance penalty? A broken master. That's infinitely bad performance.

[yuyichao]

Not yet (see comment) and no. Old julia version aren't gone yet.

Also note that master isn't completely broken (some cases works) and I've said many times that whoever want to split out part of this to another PR are welcome to do so. I've merged such PR and I've even done the work of fixing that PR and rebas this on the new master.

Also note that this version is also "broken" awaiting the HDF5 PR linked above.

[davidssmith]

Well it is broken for me, and I created a PR a long time ago that made it work for me, but it was rejected for the reason that it was incomplete, even though it fixed some things.

Don't let perfect be the enemy of the good.

[yuyichao]

I am very surprised. If you are talking about #92. I believe it's included in #100, if not, feel free to include the missing pieces.
Not being complete is not an issue. #100 is just like that. However, if it does not completely fix all issues on 1.0, it should not break 0.6.

[davidssmith]

There should be no rush to get everything working on 1.0 since

Also I want to directly disagree with this. Many packages depend on this as part of their backend. The performance of reading and writing MAT files is not as important as allowing all of those packages to upgrade to 1.0+, where they may enjoy speed improvements in execution and better likelihood of adoption. As it is now, anyone who hears about Julia, goes to download 1.0, and tries to install MAT or some package depending on MAT, will get errors. That's not the way to win hearts and minds.

[yuyichao]

You are just giving very good reason for why the release 1.0 shouldn't have been made so quickly. 0.6 is still a perfectly good release to use and start with. I know this is directly conflicting with the super loose release policy and I don't want to argue with either.

Also, I'm not looking for perfect either. It's just about not introducing regression. And if you want to know what is the right way to do this on a very much related change? JuliaLang/julia#21831 (comment) Noticed that the PRs are submitted to the packages to make sure there's no regression before the base PR is merged.

[oxinabox]

However, if it does not completely fix all issues on 1.0, it should not break 0.6.

I must be missing something.
This PR will change the minimum julia version to be 0.7
It thus cannot break julia 0.6.

If you are saying that you don't want to lose the ability to make improvements to 0.6,
then have a backports branch.
I am maintaining 2 packages that have backports branchs and it isn't that hard.

[yuyichao]

This PR will change the minimum julia version to be 0.7

Please read and quote the context.

Not being complete is not an issue. #100 is just like that. However, if it does not completely fix all issues on 1.0, it should not break 0.6.

It's specifically referring to PRs that are "not complete" and "like #100".

[babaq]

is this branch dead or not, I think it's better get merged and make it work first, and then make another PR to fix performance regression. v1.0 is relatively big change to v0.6 that's why it's better to move to v1.0 early than doing more work in v0.6. please make this work early, cause a lot of data is in MAT format.

[chriselrod]

FWIW, this branch works for me, but the merged one errors.

[jaakkor2]

Looks like Travis CI was run two months ago. For me this PR passes tests using HDF5 v0.10.4. One line fix is needed for HDF5 v0.11.0. Who is able to trigger Travis? Looks like closing and reopening 18 days ago did not do it.

[timholy]

Superseded by #110 (merged in #113)