Skip to content

Conversation

@andy-stark-redis
Copy link
Contributor

@andy-stark-redis andy-stark-redis commented Apr 10, 2025

DOC-5073

All feedback welcome. When we are happy with this, I'll use it as a model for similar pages in the other client sections, as soon as they add support for vector sets.

@andy-stark-redis andy-stark-redis requested review from a team and mortensi April 10, 2025 12:34
@andy-stark-redis andy-stark-redis self-assigned this Apr 10, 2025
Comment on lines +81 to +91
"description": """
Polish-French chemist and physicist. The only person ever to win
two Nobel prizes for two different sciences.
"""
},
"Linus Pauling": {
"born": 1901, "died": 1994,
"description": """
American chemist and peace activist. One of only two people to win two
Nobel prizes in different fields (chemistry and peace).
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems there's a contradiction here. XD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really... Marie Curie won Nobels for two different sciences (physics and chemistry). Pauling won one for chemistry and the other for peace (one science, one not). The few other people who have won two Nobels won both of them for the same subject.

Copy link
Collaborator

Choose a reason for hiding this comment

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

sips coffee Ah yes, I see now. As you were.


The first of these imports is the
`SentenceTransformer` class, which generates an embedding from a section of text.
Here, we create an instance of `SentenceTransformer` that uses the
Copy link
Collaborator

@dwdougherty dwdougherty Apr 10, 2025

Choose a reason for hiding this comment

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

The beginning of a change to first person plural, which goes against our SG. There's quite a bit of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed (I think). Good catch - I guess I got desensitised to it by reading it over and over again :-(

Copy link
Collaborator

@dwdougherty dwdougherty left a comment

Choose a reason for hiding this comment

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

Needs quite a bit of "point of view" work: 1st person plural to 2nd person. That said, I like the examples.

Copy link
Collaborator

@dwdougherty dwdougherty left a comment

Choose a reason for hiding this comment

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

LGTM.

@andy-stark-redis
Copy link
Contributor Author

Thanks @dwdougherty !

@andy-stark-redis andy-stark-redis merged commit 35de25c into main Apr 11, 2025
5 checks passed
@andy-stark-redis andy-stark-redis deleted the DOC-5073-vec-set-py-specific branch April 11, 2025 09:00
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.

3 participants