Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Const generics Improvements #980

Merged
merged 4 commits into from
Dec 6, 2021
Merged

Conversation

YuhanLiin
Copy link
Contributor

Adds the easy const generics improvements mentioned in #961. This includes implementation of IntoNdProducer, FixedInitializer, and NdIndex<IxDyn> for arrays as well as From implementations for 2D slices (slices of higher dimensions is also possible).

@bluss bluss added this to the 0.16.0 milestone Apr 13, 2021
bluss
bluss previously requested changes Apr 14, 2021
Copy link
Member

@bluss bluss left a comment

Choose a reason for hiding this comment

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

Nice work. Since this is a breaking change we need to resolve any 0.15.x changes before merging this.

src/free_functions.rs Show resolved Hide resolved
src/free_functions.rs Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
src/zip/ndproducer.rs Outdated Show resolved Hide resolved
tests/array.rs Show resolved Hide resolved
@bluss
Copy link
Member

bluss commented Apr 14, 2021

Thanks, looks good.

@bluss bluss dismissed their stale review April 14, 2021 20:15

Good changes

@YuhanLiin YuhanLiin requested a review from bluss April 16, 2021 14:33
@bluss bluss removed their request for review April 16, 2021 15:03
@bluss
Copy link
Member

bluss commented Apr 23, 2021

This is good and just waiting to be able to be integrated. After 0.15.x changes are resolved. No schedule, but it's more than a month until then.

@bluss
Copy link
Member

bluss commented Dec 3, 2021

Just to note, that it's finally time to accept breaking changes 🙂

Add From impl to convert from 2D slices to 2D views

Move NdIndex<IxDyn> implementations for arrays out of macros

Remove FixedInitializer trait

Make bounds checks in aview2 and aview_mut2 conditional on slices of ZSTs

Refactor aview2 and aview_mut2 implementations into From
use crate::iter::{Iter, IterMut};
use crate::NdIndex;
use crate::{dimension, imp_prelude::*};
Copy link
Member

Choose a reason for hiding this comment

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

I don't know what tool put this use here, but prelude-like imports should be on their own line, with a newline separating from the others.

tests/array.rs Outdated
a.remove_index(Axis(0), 1);
a.remove_index(Axis(1), 2);
assert_eq!(a.shape(), &[3, 2]);
assert_eq!(a,
array![[1, 2],
Copy link
Member

Choose a reason for hiding this comment

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

Sorry to nitpick, but when the code is arranged to be easy to read (multiline array), it should not be reformatted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have my editor set to automatically run rustfmt. I'll revert the format changes to this file.

@YuhanLiin
Copy link
Contributor Author

Why are there CI Clippy errors in code I didn't change?

@bluss
Copy link
Member

bluss commented Dec 4, 2021

clippy is using the beta branch, so it gets updates and sometimes finds new lints. Dont' bother with irrelevant clippy warnings

@YuhanLiin
Copy link
Contributor Author

CI passes for all steps except for Clippy.

@bluss bluss merged commit 6c8b821 into rust-ndarray:master Dec 6, 2021
@bluss
Copy link
Member

bluss commented Dec 6, 2021

Thanks

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants