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

Encapsulate NdNorms array. #90

Merged
merged 1 commit into from
Oct 20, 2019
Merged

Encapsulate NdNorms array. #90

merged 1 commit into from
Oct 20, 2019

Conversation

sebpuetz
Copy link
Member

@sebpuetz sebpuetz commented Oct 18, 2019

Encapsulate NdNorms array, provide interface through deref to
Array1.

I also removed the Norms trait that I didn't even know existed before. I assume that was planned as a compatibility layer if we introduced other norm chunks. Now that we're planning to stabilize the API and don't intend to add new chunks and features I think this can go away until we'd have to break the API anyways. With only the Norms trait for accessing a norm, NdNorms is useless unless `Norms' is imported.

@sebpuetz sebpuetz mentioned this pull request Oct 18, 2019
18 tasks
@RealNicolasBourbaki
Copy link

The coding-style here looks more consistent now :)

Copy link
Member

@danieldk danieldk left a comment

Choose a reason for hiding this comment

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

Some small comments.

src/chunks/norms.rs Outdated Show resolved Hide resolved
src/chunks/norms.rs Outdated Show resolved Hide resolved
src/chunks/norms.rs Outdated Show resolved Hide resolved
Copy link
Member

@danieldk danieldk left a comment

Choose a reason for hiding this comment

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

One small requested change, looks good after that.

src/embeddings.rs Outdated Show resolved Hide resolved
Encapsulate NdNorms array, provide interface through deref to
Array1.
@sebpuetz sebpuetz merged commit e78553b into master Oct 20, 2019
@sebpuetz sebpuetz deleted the encapsulate-norms branch October 20, 2019 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants