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

feat: Added simple function to mfdis.py #2056

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

camille12225
Copy link

New function returns 3 lists of integers for

  1. Stress periods in the model
  2. Timesteps in each stress period
  3. Length of each timestep

Copy link

codecov bot commented Jan 12, 2024

Codecov Report

Attention: Patch coverage is 9.09091% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 73.0%. Comparing base (96ede0d) to head (3c60197).
Report is 77 commits behind head on develop.

Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #2056     +/-   ##
=========================================
- Coverage     73.0%   73.0%   -0.1%     
=========================================
  Files          259     259             
  Lines        59298   57932   -1366     
=========================================
- Hits         43305   42294   -1011     
+ Misses       15993   15638    -355     
Files Coverage Δ
flopy/modflow/mfdis.py 89.1% <9.0%> (-2.8%) ⬇️

... and 72 files with indirect coverage changes

@jdhughes-usgs
Copy link
Contributor

You will need to relint with black (see failed test). Also to maintain code coverage you need to add a small test of your function that confirms that you get expected results for a known case.

@camille12225
Copy link
Author

Sure, I reformatted with black. I will push again when I add the CI test.

stp = []
perlen = []
for iper in range(self.nper):
per.append(iper + 1) # Use 1-based indexing
Copy link
Member

Choose a reason for hiding this comment

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

it's probably best to return 0-based per flopy3+ convention

@wpbonelli wpbonelli added this to the 3.7.0 milestone May 3, 2024
@wpbonelli
Copy link
Member

@camille12225 develop has moved a lot and linting has switched to ruff since this was opened, in case you need to again it can be done with ruff check . and ruff format . from the project root. It looks like this PR doesn't need a rebase before merging though.

@wpbonelli wpbonelli modified the milestones: 3.7.0, 3.8.0 May 23, 2024
@wpbonelli wpbonelli modified the milestones: 3.8.0, 3.9.0 Aug 5, 2024
@wpbonelli wpbonelli modified the milestones: 3.9, 3.10 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants