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

Adds missing unit tests, end to end testing of the daily/monthly CLI and some code cleanups #36

Merged
merged 28 commits into from
May 8, 2023

Conversation

stijnvanhoey
Copy link
Collaborator

@stijnvanhoey stijnvanhoey commented Mar 27, 2023

@peterdesmet to support the 'all' option, one can use the 0 option (to keep it an integer)

❯ vph5_to_vpts --help
Usage: vph5_to_vpts [OPTIONS]

  Convert and aggregate h5 vp files to daily/monthly vpts files on s3 bucket

  Check the latest modified h5 files on the s3 bucket using an s3 inventory,
  convert all ODIM hdf5 profiles files for the days with modified files to the
  vpts-csv standard and upload the vpts-csv daily/monthly files to s3.

Options:
  --modified-days-ago INTEGER  All bucket files with a modified date between
                               now and N modified_days_ago will be taken into
                               account for the recreation of daily/monthly
                               files. If 0,all nucket files will be taken into
                               account for the recreation.
  --aws-profile TEXT           Optionally, define the AWS profile used to run
                               the command for interaction with the AWS s3
                               bucket.
  --help                       Show this message and exit.

With This PR, the targeted set of functionalities is provided. I marked as draft as I do have to fix an issue with the sphinx documentation building. I'll do that asap.

@stijnvanhoey stijnvanhoey marked this pull request as draft March 27, 2023 22:28
@coveralls
Copy link

coveralls commented Mar 27, 2023

Pull Request Test Coverage Report for Build 4561907488

  • 58 of 59 (98.31%) changed or added relevant lines in 6 files are covered.
  • 7 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+36.7%) to 88.018%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/vptstools/bin/vph5_to_vpts.py 16 17 94.12%
Files with Coverage Reduction New Missed Lines %
src/vptstools/vpts_csv.py 7 76.99%
Totals Coverage Status
Change from base Build 4409840957: 36.7%
Covered Lines: 392
Relevant Lines: 412

💛 - Coveralls

@peterdesmet
Copy link
Member

Sounds good, nice coverage!

please correct typos to:

All bucket files with a modified date between now and N modified-days-ago will be taken into account for the recreation of daily/monthly files. If 0, all bucket files will be taken into account for the recreation.

@peterdesmet peterdesmet self-requested a review March 29, 2023 09:21
Copy link
Member

@peterdesmet peterdesmet left a comment

Choose a reason for hiding this comment

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

@stijnvanhoey I made some changes to this in a separate PR #37, so you can review those better.

docs/conf.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Editorial edits to cleanup branch
@peterdesmet peterdesmet self-requested a review March 30, 2023 07:20
@stijnvanhoey stijnvanhoey marked this pull request as ready for review May 8, 2023 12:53
@stijnvanhoey stijnvanhoey merged commit d5e5fae into main May 8, 2023
@stijnvanhoey stijnvanhoey deleted the cleanup branch May 8, 2023 14:48
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.

None yet

3 participants