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

Clippy fixes. #846

Merged
merged 15 commits into from
Dec 15, 2021
2 changes: 1 addition & 1 deletion .github/workflows/docs-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
- name: Install Python
uses: actions/setup-python@v1
with:
python-version: 3.6
python-version: 3.9

- name: Install dependencies
run: pip install sphinx sphinx_rtd_theme setuptools-rust
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/docs-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
- name: Install Python
uses: actions/setup-python@v1
with:
python-version: 3.6
python-version: 3.9

- name: Install Python dependencies
run: pip install sphinx sphinx_rtd_theme setuptools-rust
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/python-release-conda.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest, macos-latest]
python: [3.6, 3.7, 3.8, 3.9]
python: [3.7, 3.8, 3.9]
defaults:
run:
shell: bash -l {0}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/python-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
runs-on: windows-latest
strategy:
matrix:
python: [3.6, 3.7, 3.8, 3.9]
python: [3.7, 3.8, 3.9]
steps:
- name: Checkout repository
uses: actions/checkout@v1
Expand Down Expand Up @@ -76,7 +76,7 @@ jobs:
strategy:
matrix:
os: [windows-latest, macos-10.15]
python: [3.6, 3.7, 3.8, 3.9]
python: [3.7, 3.8, 3.9]
steps:
- name: Checkout repository
uses: actions/checkout@v1
Expand Down
36 changes: 18 additions & 18 deletions .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
runs-on: windows-latest
strategy:
matrix:
python: [3.6, 3.7, 3.8, 3.9]
python: [3.7, 3.8, 3.9]
steps:
- name: Checkout repository
uses: actions/checkout@v1
Expand All @@ -34,7 +34,7 @@ jobs:


- name: Install Python
uses: actions/setup-python@v1
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python }}
architecture: x86
Expand All @@ -45,6 +45,7 @@ jobs:
command: build
args: --manifest-path ./bindings/python/Cargo.toml


build_and_test:
name: Check everything builds & tests
runs-on: ${{ matrix.os }}
Expand All @@ -54,27 +55,25 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v1


- name: Install Rust
uses: actions-rs/toolchain@v1
with:
toolchain: stable
components: rustfmt, clippy

# # Necessary for now for the cargo cache: https://github.com/actions/cache/issues/133#issuecomment-599102035
# - run: sudo chown -R $(whoami):$(id -ng) ~/.cargo/

# - name: Cache Cargo Registry
# uses: actions/cache@v1
# with:
# path: ~/.cargo/registry
# key: ${{ runner.os }}-cargo-registry-${{ hashFiles('**/Cargo.lock') }}
- name: Cache Cargo Registry
uses: actions/cache@v1
with:
path: ~/.cargo/registry
key: ${{ runner.os }}-cargo-registry-${{ hashFiles('**/Cargo.lock') }}

# - name: Cache Cargo Build Target
# uses: actions/cache@v1
# with:
# path: ./bindings/python/target
# key: ${{ runner.os }}-cargo-python-build-${{ hashFiles('**/Cargo.lock') }}
- name: Cache Cargo Build Target
uses: actions/cache@v1
with:
path: ./bindings/python/target
key: ${{ runner.os }}-cargo-python-build-${{ hashFiles('**/Cargo.lock') }}

- name: Lint with RustFmt
uses: actions-rs/cargo@v1
Expand All @@ -90,17 +89,18 @@ jobs:
args: --manifest-path ./bindings/python/Cargo.toml --all-targets --all-features -- -D warnings

- name: Install Python
uses: actions/setup-python@v1
uses: actions/setup-python@v2
with:
python-version: 3.6
python-version: 3.9
architecture: "x64"

- name: Install
working-directory: ./bindings/python
run: |
python -m venv .env
source .env/bin/activate
pip install pytest requests setuptools_rust numpy "pyarrow<3.0.0" datasets
pip install -U pip
pip install pytest requests setuptools_rust numpy pyarrow datasets
python setup.py develop

- name: Check style
Expand Down
16 changes: 4 additions & 12 deletions bindings/node/native/src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,7 @@ impl BpeOptions {
fn bpe_init(mut cx: FunctionContext) -> JsResult<JsModel> {
let vocab = cx.extract::<Vocab>(0)?;
let merges = cx.extract::<Merges>(1)?;
let options = cx
.extract_opt::<BpeOptions>(2)?
.unwrap_or_else(BpeOptions::default);
let options = cx.extract_opt::<BpeOptions>(2)?.unwrap_or_default();

let mut builder = tk::models::bpe::BPE::builder().vocab_and_merges(vocab, merges);
builder = options.apply_to_bpe_builder(builder);
Expand Down Expand Up @@ -251,9 +249,7 @@ impl WordPieceOptions {
/// })
fn wordpiece_init(mut cx: FunctionContext) -> JsResult<JsModel> {
let vocab = cx.extract::<HashMap<String, u32>>(0)?;
let options = cx
.extract_opt::<WordPieceOptions>(1)?
.unwrap_or_else(WordPieceOptions::default);
let options = cx.extract_opt::<WordPieceOptions>(1)?.unwrap_or_default();

let mut builder = tk::models::wordpiece::WordPiece::builder().vocab(vocab);
builder = options.apply_to_wordpiece_builder(builder);
Expand Down Expand Up @@ -320,9 +316,7 @@ impl WordLevelOptions {
/// }, callback)
fn wordlevel_init(mut cx: FunctionContext) -> JsResult<JsModel> {
let vocab = cx.extract::<HashMap<String, u32>>(0)?;
let options = cx
.extract_opt::<WordLevelOptions>(1)?
.unwrap_or_else(WordLevelOptions::default);
let options = cx.extract_opt::<WordLevelOptions>(1)?.unwrap_or_default();

let mut builder = tk::models::wordlevel::WordLevel::builder().vocab(vocab);
builder = options.apply_to_wordlevel_builder(builder);
Expand Down Expand Up @@ -378,9 +372,7 @@ struct UnigramOptions {
/// })
fn unigram_init(mut cx: FunctionContext) -> JsResult<JsModel> {
let vocab = cx.extract::<Vec<(String, f64)>>(0)?;
let options = cx
.extract_opt::<UnigramOptions>(1)?
.unwrap_or_else(UnigramOptions::default);
let options = cx.extract_opt::<UnigramOptions>(1)?.unwrap_or_default();

let unigram = tk::models::unigram::Unigram::from(vocab, options.unk_id)
.map_err(|e| Error(e.to_string()))?;
Expand Down
2 changes: 1 addition & 1 deletion bindings/node/native/src/normalizers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl Default for BertNormalizerOptions {
fn bert_normalizer(mut cx: FunctionContext) -> JsResult<JsNormalizer> {
let options = cx
.extract_opt::<BertNormalizerOptions>(0)?
.unwrap_or_else(BertNormalizerOptions::default);
.unwrap_or_default();

let mut normalizer = JsNormalizer::new::<_, JsNormalizer, _>(&mut cx, vec![])?;
let guard = cx.lock();
Expand Down
6 changes: 3 additions & 3 deletions bindings/node/native/src/tasks/tokenizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub enum EncodeTask<'s> {
}

pub enum EncodeOutput {
Single(Encoding),
Single(Box<Encoding>),
Copy link
Member

Choose a reason for hiding this comment

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

Is Box needed because Javascript? 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, clippy yells becuse that variant is 240 bytes long (pretty long for an enum). Setting it as a box just uses a pointer instead.

Batch(Vec<Encoding>),
}

Expand All @@ -35,7 +35,7 @@ impl Task for EncodeTask<'static> {
*add_special_tokens,
)
.map_err(|e| format!("{}", e))
.map(EncodeOutput::Single)
.map(|item| EncodeOutput::Single(Box::new(item)))
}
EncodeTask::Batch(worker, input, add_special_tokens) => {
let mut input: Option<Vec<EncodeInput>> =
Expand Down Expand Up @@ -65,7 +65,7 @@ impl Task for EncodeTask<'static> {
let mut js_encoding = JsEncoding::new::<_, JsEncoding, _>(&mut cx, vec![])?;
// Set the actual encoding
let guard = cx.lock();
js_encoding.borrow_mut(&guard).encoding = Some(encoding);
js_encoding.borrow_mut(&guard).encoding = Some(*encoding);
Ok(js_encoding.upcast())
}
EncodeOutput::Batch(encodings) => {
Expand Down
4 changes: 2 additions & 2 deletions bindings/node/native/src/tokenizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ declare_types! {
let content = cx.extract::<String>(0)?;
let special = cx.extract::<bool>(1)?;
let token = cx.extract_opt::<AddedTokenOptions>(2)?
.unwrap_or_else(AddedTokenOptions::default)
.unwrap_or_default()
.into_added_token(content, special);

Ok(AddedToken { token })
Expand Down Expand Up @@ -1034,7 +1034,7 @@ pub fn tokenizer_from_pretrained(mut cx: FunctionContext) -> JsResult<JsTokenize
let s = cx.extract::<String>(0)?;
let mut p: tk::FromPretrainedParameters = cx
.extract_opt::<FromPretrainedParametersJs>(1)?
.unwrap_or_else(FromPretrainedParametersJs::default)
.unwrap_or_default()
.into();

p.user_agent = [("bindings", "Node.js"), ("version", crate::VERSION)]
Expand Down
Loading