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

Feature/ras 1d #55

Merged
merged 38 commits into from
Jun 24, 2024
Merged

Feature/ras 1d #55

merged 38 commits into from
Jun 24, 2024

Conversation

sray014
Copy link
Contributor

@sray014 sray014 commented Jun 14, 2024

Adds functions to export the following 1D Output data from HDF files as well as unit tests for each function:

  • Cross Sections
  • River Reaches
  • Cross Section Velocity
  • Steady Flow Names
  • Cross Section Area
  • Encroachment Information
  • Cross Section Elevation
  • Cross Section WSEL Information
  • Cross Section Energy Grade
  • Cross Section Flow

@sray014 sray014 requested a review from slawler June 14, 2024 17:14
Copy link

codecov bot commented Jun 15, 2024

Codecov Report

Attention: Patch coverage is 95.14563% with 5 lines in your changes missing coverage. Please review.

Files Coverage Δ
src/rashdf/plan.py 99.02% <100.00%> (+0.17%) ⬆️
src/rashdf/geom.py 84.51% <91.07%> (+3.46%) ⬆️

@sray014
Copy link
Contributor Author

sray014 commented Jun 19, 2024

@thwllms I assume the version in the pyproject.toml will need to be bumped up to 0.3.1?

@thwllms
Copy link
Contributor

thwllms commented Jun 19, 2024

@thwllms I assume the version in the pyproject.toml will need to be bumped up to 0.3.1?

I'd bump to 0.4.0 since this adds new features. A +0.0.1 bump would be appropriate for a bugfixes-only update.

Copy link
Member

@slawler slawler left a comment

Choose a reason for hiding this comment

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

@sray014 please update the version in the project.toml

@slawler
Copy link
Member

slawler commented Jun 20, 2024

@thwllms Looks like the issues have been resolved, I approved, will wait for you to merge once you've had a chance to review.

XsSteadyAdditionalVar.ENCROACHMENT_STATION_RIGHT,
XsSteadyAdditionalVar.AREA_INEFFECTIVE_TOTAL,
XsSteadyAdditionalVar.VELOCITY_TOTAL,
]
Copy link
Contributor

@thwllms thwllms Jun 23, 2024

Choose a reason for hiding this comment

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

  • The XS_STEADY_OUTPUT and XS_STEADY_ADDITIONAL constants here are redundant since they both contain the full list of variables in each corresponding enum. The other lists of specific enum values above were created because there are places where only some of the enum values are relevant. You could either combine the enums into a single XsSteadyOutputVar enum and create an XS_STEADY_OUTPUT_ADDITIONAL list that contains only "additional" vars, or simply remove the XS_STEADY_OUTPUT and XS_STEADY_ADDITIONAL vars.
  • Might as well add the full list of additional output variables? Creating a named method for each is probably overkill, but having those variables in the enum would allow us to access that data via steady_profile_xs_output

Copy link
Contributor

@thwllms thwllms left a comment

Choose a reason for hiding this comment

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

LGTM, just the one comment about the enums/constants in plan.py. Don't see a need to request re-review.

@sray014
Copy link
Contributor Author

sray014 commented Jun 24, 2024

@thwllms does fiona need to be added to the pyproject dependencies for those checks to work? Not sure why it worked on the previous commit and not this one since nothing changed that deals with that.

@thwllms
Copy link
Contributor

thwllms commented Jun 24, 2024

@thwllms does fiona need to be added to the pyproject dependencies for those checks to work? Not sure why it worked on the previous commit and not this one since nothing changed that deals with that.

Looks like this is happening because GeoPandas 1.0.0 was just released a few hours ago and I guess it doesn't rely on Fiona as an explicit dependency anymore. We were previously relying on version ~0.14. For now let's pin GeoPandas to version number 0.14 and create an issue to upgrade to 1.0.0.

In pyproject.toml:

name = "rashdf"
description = "Read data from HEC-RAS HDF files."
readme = "README.md"
classifiers = [
    "Development Status :: 4 - Beta",
    "Intended Audience :: Developers",
    "License :: OSI Approved :: MIT License",
    "Programming Language :: Python :: 3",
    "Programming Language :: Python :: 3.9",
    "Programming Language :: Python :: 3.10",
    "Programming Language :: Python :: 3.11",
    "Programming Language :: Python :: 3.12",
]
version = "0.4.0"
dependencies = ["h5py", "geopandas>=0.14,<0.15", "pyarrow", "xarray"]

https://pypi.org/project/geopandas/#history

@sray014 sray014 merged commit a5a7e2e into main Jun 24, 2024
5 checks passed
@sray014 sray014 deleted the Feature/ras_1d branch June 24, 2024 15:29
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.

4 participants