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

set persist as option via argument #319

Merged
merged 11 commits into from
Feb 2, 2023
Merged

Conversation

Mikejmnez
Copy link
Collaborator

This closes issue #315 . Whether the transformation is persisted or not, is controlled via a True or False (it is False by default) argument that is passed to llc_rearrange.py via oceanspy.subsample.cutout. This is

cut_od = od.subsample.cutout(XRange, YRange)

performs a cutout defined by XRange and YRange, where the transformation is NOT persisted.

To persist the transformation before the cutout:

cut_od = od.subsample.cutout(XRange, YRange, persist=True)

@codecov
Copy link

codecov bot commented Feb 2, 2023

Codecov Report

Merging #319 (df7b711) into main (df54edf) will decrease coverage by 0.25%.
The diff coverage is 37.50%.

@@            Coverage Diff             @@
##             main     #319      +/-   ##
==========================================
- Coverage   95.74%   95.50%   -0.25%     
==========================================
  Files           9        9              
  Lines        3928     3936       +8     
  Branches      850      854       +4     
==========================================
- Hits         3761     3759       -2     
- Misses         97      102       +5     
- Partials       70       75       +5     
Flag Coverage Δ
unittests 95.50% <37.50%> (-0.25%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
oceanspy/_oceandataset.py 97.12% <ø> (ø)
oceanspy/animate.py 98.01% <ø> (ø)
oceanspy/compute.py 98.81% <ø> (ø)
oceanspy/plot.py 97.52% <ø> (ø)
oceanspy/subsample.py 98.04% <ø> (ø)
oceanspy/llc_rearrange.py 87.30% <37.50%> (-1.30%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Mikejmnez
Copy link
Collaborator Author

Mikejmnez commented Feb 2, 2023

This is the error being described by pre-commit.ci

error

This is defined within the pre-commit-config.yaml file:

- repo: https://github.com/nbQA-dev/nbQA
  rev: 1.6.1
  hooks:
  - id: nbqa-black
    additional_dependencies: [black]
    args: [--nbqa-mutate]
  - id: nbqa-isort
    additional_dependencies: [isort]
    args: [--nbqa-mutate]

When running pre-commit run --all on my local fork, everything runs fine. I have version 3.0.2 of pre-commit

@ThomasHaine
Copy link
Collaborator

What's the version of the automated pre-commit.ci? Can the failure be corrected by updating to 3.0.2?

Also, can you share a notebook that demos both versions of persist={True,False}. I want to experiment.

@Mikejmnez
Copy link
Collaborator Author

Got it. I had to do a series of updates on pre-commit so it would show the same errors on my local computer/workstation as in here (snapshot above). It finally did and was able to fix those two files.

I did:

pre-commit autoupdate --bleeding-edge --repo https://github.com/pre-commit/pre-commit-hooks
pre-commit clean

and finally

pre-commit run --all

@Mikejmnez Mikejmnez merged commit 0d028fd into hainegroup:main Feb 2, 2023
@Mikejmnez Mikejmnez deleted the pers branch February 2, 2023 15:42
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.

2 participants