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

Fix #433 - Gis files written with column title 'name' #435

Merged
merged 10 commits into from
Aug 12, 2024

Conversation

angusmcb
Copy link
Contributor

Summary

Changes default name column from 'index' to 'name' and removes a couple of instances where it was assumed that there was a column 'index'.

Fixes #433

Tests and documentation

n/a - not a new feature

Acknowledgement

By contributing to this software project, I acknowledge that I have reviewed the software quality assurance guidelines and that my contributions are submitted under the Revised BSD License.

@kaklise kaklise requested a review from kbonney July 30, 2024 17:54
Copy link
Collaborator

@kbonney kbonney left a comment

Choose a reason for hiding this comment

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

All changes make sense and contribute to more consistent index naming within WNTR functionality. These changes do change default behavior of WNTR, so it could potentially break code that relies on the default index name being "index" - we'll want to call this out in our next release.

Currently the doctests for the gis module are failing since they expect dataframes without an index name, but we are now naming it "name". @angusmcb, can you address this test failure?

@coveralls
Copy link

Coverage Status

coverage: 87.917% (+0.01%) from 87.907%
when pulling 4247ef3 on angusmcb:gis_name
into eeb16c3 on USEPA:main.

@kbonney
Copy link
Collaborator

kbonney commented Aug 8, 2024

Added additional tests to verify desired index name behavior across GIS functionality. Also addressed some doc testing issues that arose as a result of the index name changes.

@angusmcb
Copy link
Contributor Author

Thanks @kbonney, sorry I couldn't get to this in time!

@kbonney kbonney merged commit 31004a9 into USEPA:main Aug 12, 2024
40 checks passed
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.

GIS files written with wrong column title for 'name'
3 participants