Skip to content

[uss_qualifier/resource] Add base class for geospatial modifier#1467

Open
the-glu wants to merge 1 commit into
interuss:mainfrom
Orbitalize:1458_geospatial_modifier
Open

[uss_qualifier/resource] Add base class for geospatial modifier#1467
the-glu wants to merge 1 commit into
interuss:mainfrom
Orbitalize:1458_geospatial_modifier

Conversation

@the-glu
Copy link
Copy Markdown
Contributor

@the-glu the-glu commented May 19, 2026

Continuing on #1465 for #1458, this PR introduce:

  • GeospatialResource, as suggested to ease implementation of
  • GeospatialModifier, allowing a resource to generate unique 'Geospatial' resources

Include simple unit tests with a TestSquareResource.

@the-glu the-glu changed the title 1458 geospatial modifier [uss_qualifier/resource] Add base class for geospatial modifier May 19, 2026
@BenjaminPelletier
Copy link
Copy Markdown
Member

I think I misread #1465 and would like to adjust it before looking closely at this one. The issue is that I would expect "resource modifier" to mean something that modifies resources, but the more important aspect of ResourceModifier in #1465 is that it presents as the modified resource itself, so it seems like a better name would be "IndexAdjustableResource". That's important because in this PR we have GeospatialModifierResource and with a name like that, I would expect it to handle any geospatial modification we might want to make to resources. Instead, it's actually just one particular way to offset GeospatialResources. With AdjustableResource, I think it would be more obvious that a name like GeospatiallyOffsetResource is probably more descriptive than GeospatialModifierResource. I think this is important because this is an external surface of the product that users will directly see and touch. I'll propose an adjustment PR.

@the-glu
Copy link
Copy Markdown
Contributor Author

the-glu commented May 20, 2026

I think I misread #1465 and would like to adjust it before looking closely at this one. The issue is that I would expect "resource modifier" to mean something that modifies resources, but the more important aspect of ResourceModifier in #1465 is that it presents as the modified resource itself, so it seems like a better name would be "IndexAdjustableResource".

Ok, to do that (behaving as the modified resource, witch it doesn't do for now) we can just have it expose the underlying type as well + proxy with getattr()/setattr() attributes to the underling resources.

Let me know, I can also do the change :)

@the-glu the-glu force-pushed the 1458_geospatial_modifier branch from 420725a to 5cb5a06 Compare May 27, 2026 15:33
@the-glu
Copy link
Copy Markdown
Contributor Author

the-glu commented May 27, 2026

@BenjaminPelletier I pre-rebased that one on top of #1474

Not sure about naming for this base class, I will update next ones in the chain when that one seems ok

@BenjaminPelletier
Copy link
Copy Markdown
Member

This branch currently has merge conflicts which makes the diffs hard to interpret; can we merge main into this branch or rebase the changes onto main?

@the-glu the-glu force-pushed the 1458_geospatial_modifier branch from 5cb5a06 to d10f877 Compare June 3, 2026 19:34
@the-glu
Copy link
Copy Markdown
Contributor Author

the-glu commented Jun 3, 2026

This branch currently has merge conflicts which makes the diffs hard to interpret; can we merge main into this branch or rebase the changes onto main?

Rebased, not sure why it diverged again

Comment thread monitoring/uss_qualifier/resources/modifiers.py Outdated
Comment thread monitoring/uss_qualifier/resources/geospatial.py
Comment thread monitoring/uss_qualifier/resources/modifiers.py Outdated
Comment thread monitoring/uss_qualifier/resources/geospatial.py
@the-glu the-glu force-pushed the 1458_geospatial_modifier branch 3 times, most recently from 89fbdf5 to 8f7b469 Compare June 4, 2026 06:42
@the-glu the-glu force-pushed the 1458_geospatial_modifier branch from 8f7b469 to 34b5e74 Compare June 4, 2026 06:48
@the-glu
Copy link
Copy Markdown
Contributor Author

the-glu commented Jun 4, 2026

@BenjaminPelletier thanks for the review, everything should be fixed :)

pass


class GeospatialModifierSpecification(ImplicitDict):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't renamed this one as it was not in the suggestion, should we as well?

@the-glu the-glu requested a review from BenjaminPelletier June 4, 2026 06:54
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.

2 participants