Skip to content

Eliminate OffsetArrays dependency#17

Merged
timholy merged 1 commit intomasterfrom
teh/compat
Jan 20, 2020
Merged

Eliminate OffsetArrays dependency#17
timholy merged 1 commit intomasterfrom
teh/compat

Conversation

@timholy
Copy link
Copy Markdown
Member

@timholy timholy commented Jan 19, 2020

This collect call handles offset axes quite generally, so the specialized method is mostly redundant. It does, however, reduce memory consumption, so I decided to keep it but put it behind a @require. Since we're already using Requires in this package, that doesn't seem like unnecessary overhead.

The main advantage of eliminating unnecessary dependencies is less CompatHelper busy work.

Copy link
Copy Markdown
Member

@johnnychen94 johnnychen94 left a comment

Choose a reason for hiding this comment

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

Since almost all packages under JuliaImages requires OffsetArray, we shouldn't expect any performance boost in Images.jl side by doing this. But yes, in ImageShow.jl side, it requires less.

P.S. I was doing a Julia installer and release mirror project and thus missed some recent activities. It might take a few more days.

@timholy timholy changed the title RFC: eliminate OffsetArrays dependency Eliminate OffsetArrays dependency Jan 20, 2020
@timholy timholy merged commit d24cbfb into master Jan 20, 2020
@timholy timholy deleted the teh/compat branch January 20, 2020 09:05
@johnnychen94 johnnychen94 mentioned this pull request Mar 14, 2020
1 task
johnnychen94 added a commit that referenced this pull request Mar 16, 2020
this commit reverts #17 because

* OffsetArrays has 1.0 release; CompatHelper notification would
  be less frequently
* ImageCore also depends on OffsetArrays; Requires doesn't help in
  saving the loading time
johnnychen94 added a commit that referenced this pull request Mar 16, 2020
this commit reverts #17 because

* OffsetArrays has 1.0 release; CompatHelper notification would be less frequent
* ImageCore also depends on OffsetArrays; Requires doesn't help in saving the loading time
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

Successfully merging this pull request may close these issues.

2 participants