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.

Sign up for GitHub

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

Jump to bottom

Refactor lexeme mem passing #12125

Merged
merged 4 commits into from
Jan 25, 2023
Merged

Refactor lexeme mem passing #12125

merged 4 commits into from
Jan 25, 2023

Conversation

polm
Copy link
Contributor

@polm polm commented Jan 19, 2023 •
edited
Loading

Description

This changes the _new_lexeme method in the vocab to not take a memory pool as an argument. The passed value has not been used since #6809.

I am having some local test failures, but they are related to my thinc version, and don't seem to have anything to do with the changes in this PR. (On review, this seems to be normal because Thinc is currently using a dev version.) This is not an issue in CI.

Types of change

minor refactor

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

@polm polm added the 🔜 v4.0 Related to upcoming v4.0 label Jan 19, 2023
@polm polm marked this pull request as ready for review January 19, 2023 07:51
shadeMe
shadeMe reviewed Jan 20, 2023
View reviewed changes
polm added 3 commits January 23, 2023 13:12
@polm
Remove unused mem from function args
ea81e74 
Two methods calling _new_lexeme, get and get_by_orth, took mem arguments
just to call the internal method. That's no longer necessary, so this
cleans it up.
@polm
prettier formatting
89642ae
@polm
Remove more unused mem args
b64cbe5
@polm
Copy link
Contributor Author

polm commented Jan 23, 2023

Good point about removing the mem argument from the calling methods. The methods involved are used in many places, so the changes were pretty extensive, but I think this should be resovled now.

@polm polm merged commit de360bc into explosion:v4 Jan 25, 2023
jordankanter pushed a commit to jordankanter/spaCy that referenced this pull request Mar 14, 2024
@polm @jordankanter
Refactor lexeme mem passing (explosion#12125)
a91494f 
* Don't pass mem pool to new lexeme function

* Remove unused mem from function args

Two methods calling _new_lexeme, get and get_by_orth, took mem arguments
just to call the internal method. That's no longer necessary, so this
cleans it up.

* prettier formatting

* Remove more unused mem args
jordankanter pushed a commit to jordankanter/spaCy that referenced this pull request Mar 14, 2024
@polm @jordankanter
Refactor lexeme mem passing (explosion#12125)
a716173 
* Don't pass mem pool to new lexeme function

* Remove unused mem from function args

Two methods calling _new_lexeme, get and get_by_orth, took mem arguments
just to call the internal method. That's no longer necessary, so this
cleans it up.

* prettier formatting

* Remove more unused mem args
jordankanter pushed a commit to jordankanter/spaCy that referenced this pull request Mar 29, 2024
@polm @jikanter
Refactor lexeme mem passing (explosion#12125)
6ba4d8c 
* Don't pass mem pool to new lexeme function

* Remove unused mem from function args

Two methods calling _new_lexeme, get and get_by_orth, took mem arguments
just to call the internal method. That's no longer necessary, so this
cleans it up.

* prettier formatting

* Remove more unused mem args
jordankanter pushed a commit to jordankanter/spaCy that referenced this pull request Apr 17, 2024
@polm @jikanter
Refactor lexeme mem passing (explosion#12125)
fa4d241 
* Don't pass mem pool to new lexeme function

* Remove unused mem from function args

Two methods calling _new_lexeme, get and get_by_orth, took mem arguments
just to call the internal method. That's no longer necessary, so this
cleans it up.

* prettier formatting

* Remove more unused mem args
jordankanter pushed a commit to jordankanter/spaCy that referenced this pull request May 10, 2024
@polm @jikanter
Refactor lexeme mem passing (explosion#12125)
05c0a1e 
* Don't pass mem pool to new lexeme function

* Remove unused mem from function args

Two methods calling _new_lexeme, get and get_by_orth, took mem arguments
just to call the internal method. That's no longer necessary, so this
cleans it up.

* prettier formatting

* Remove more unused mem args
jordankanter pushed a commit to jordankanter/spaCy that referenced this pull request May 10, 2024
@polm @jikanter
Refactor lexeme mem passing (explosion#12125)
7ab2dc8 
* Don't pass mem pool to new lexeme function

* Remove unused mem from function args

Two methods calling _new_lexeme, get and get_by_orth, took mem arguments
just to call the internal method. That's no longer necessary, so this
cleans it up.

* prettier formatting

* Remove more unused mem args
jordankanter pushed a commit to jordankanter/spaCy that referenced this pull request May 21, 2024
@polm @jikanter
Refactor lexeme mem passing (explosion#12125)
1e50ab9 
* Don't pass mem pool to new lexeme function

* Remove unused mem from function args

Two methods calling _new_lexeme, get and get_by_orth, took mem arguments
just to call the internal method. That's no longer necessary, so this
cleans it up.

* prettier formatting

* Remove more unused mem args
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shadeMe shadeMe shadeMe approved these changes

Assignees
No one assigned
Labels
🔜 v4.0 Related to upcoming v4.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@polm @shadeMe