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

d.vect.thematic: Do not show decimal places for ints #3096

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

wenzeslaus
Copy link
Member

@wenzeslaus wenzeslaus commented Jul 19, 2023

Number of minimal decimal places is fixed to 2 in the legend generated by d.vect.thematic. This adds logic which changes the number of decimal places to 0 when the column type is integer (DB_C_TYPE_INT).

This does not let user decide the number, but it covers more cases than the current code and it prepares a way for making it parametrized in the future.

Number of minimal decimal places is fixed to 2 in the legend generated by d.vect.thematic. This adds logic which changes the number of decimal places to 0 when the column type is integer (DB_C_TYPE_INT).

This does not let user decide the number, but it covers more cases and it prepares a way for making it parametrized in the future.
@neteler neteler added this to the 8.4.0 milestone Aug 16, 2023
@landam landam added the vector Related to vector data processing label Nov 19, 2023
@cmbarton
Copy link
Contributor

This will be a nice update if the module can be made to work with 8.4

@wenzeslaus wenzeslaus enabled auto-merge (squash) January 30, 2024 17:38
@github-actions github-actions bot added C Related code is in C module display and removed vector Related to vector data processing labels Jan 30, 2024
@wenzeslaus
Copy link
Member Author

The current CI checks are now running on this and the PR is now set to auto-merge when it passes the requirements. With our new system with required reviews, it needs an approving review from a maintainer.

In this new system, users and contributors without write access can help by leaving a reviews for whatever they feel qualified to judge, e.g. after testing the change.

@echoix
Copy link
Member

echoix commented Jan 30, 2024

Depending on how it was configured exactly, someone without write access can leave reviews, but they'll be gray instead of green, and won't "count" on the required reviews. However, I'm open to patch this and approve a PR if it is approved by such a person that would be a better reference than me, to unblock those rare cases.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/about-pull-request-reviews#:~:text=If%20your%20repository,can%20be%20merged.

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

Tested and it works

@wenzeslaus wenzeslaus merged commit 8a85c75 into OSGeo:main Jan 30, 2024
25 checks passed
@wenzeslaus wenzeslaus deleted the d_vect_thematic-ints-in-legend branch January 30, 2024 21:53
@wenzeslaus
Copy link
Member Author

...someone without write access can leave reviews, but they'll be gray instead of green, and won't "count" on the required reviews. However, I'm open to patch this and approve a PR if it is approved by such a person that would be a better reference than me, to unblock those rare cases.

I think that should be our approach when applicable. We have a lot of skillful users/contributors/testers who can evaluate the functionality, sometimes even the code, so it would be great to have and use their input.

Depending on how it was configured exactly...

As far as I can tell, that's the only configuration and that makes sense to me.

jadenabrams100 pushed a commit to ncsu-csc472-spring2024/grass-CI-playground that referenced this pull request Feb 21, 2024
Number of minimal decimal places is fixed to 2 in the legend generated by d.vect.thematic. This adds logic which changes the number of decimal places to 0 when the column type is integer (DB_C_TYPE_INT).

This does not let user decide the number, but it covers more cases than the current code and it prepares a way for making it parametrized in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C display module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants