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

Added the easily fixed stuff that shouldn't need discussion #23

Merged
merged 8 commits into from
Apr 15, 2024

Conversation

abodeuis
Copy link
Contributor

This PR is the easy fixes that are just getting stuff functional.

All Fixes were only to Polygon Feature results

  • Renamed PolygonLegendAndFeauturesResult to PolygonLegendAndFeatureResult to correct spelling
  • Added "label" field to PolygonLegendAndFeatureResult
  • Fixed type of legend_bbox field to actually be able to send bounding box data
  • For all optional fields added = None or default=None so that this doesn't have to be specified in the constructor.
  • Cleaned the formatting of PolygonLegendAndFeatureResult up a bit and added some descriptions.

Hopefully these parts at least can be merged immediately as they are just fixes and shouldn't need discussion

@jgawrilo
Copy link
Contributor

Only thing I'd push back against is the renaming to PolygonLegendAndFeatureResult instead of PolygonLegendAndFeaturesResult. I believe we could keep the plural form to ensure folks know this stored multiple feature features in one result. @marshHawk4 thoughts?

@abodeuis
Copy link
Contributor Author

Create a 2 point bounding box and a polygon bounding box field for legend bounding boxes

@abodeuis
Copy link
Contributor Author

I left legend_bbox as a 2 point bounding box and added legend_contour field which will be the polygon bounding box. @marshHawk4 Should be ready for a merge if you want to go ahead and review

description="""The extacted bounding box of the legend item.
Column value from left, row value from bottom."""
default=None,
description="The rough 2 point bounding box of the map units label.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add in example like [x_min, y_min, x_max, y_max]
Since the order of these matter and I have seen other formats for bbox that are not this exact one.

@marshHawk4
Copy link
Collaborator

PolygonLegendAndFeatureResult needs to be PolygonLegendAndFeaturesResult

@marshHawk4 marshHawk4 merged commit f71cc14 into main Apr 15, 2024
1 check passed
@marshHawk4 marshHawk4 deleted the abode_feature_result_fixes branch April 15, 2024 19:40
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.

3 participants