[Requirements] Naive Implementation, Requirement#2, "Get Image Close To", Structural Proposal, Introduction For Basic Design Ideas

[Rubix982]

Naive Implementation, Requirement#2, "Get Image Close To", Structural Proposal, Introduction For Basic Design Ideas

Pull Request Type

2. 🏆 Enhancements - 2nd requirement fulfillment, naive implementation. Structural improvements with conventional standards

3. 📜 Documentation - Improved documentation wherever possible

4. 🐛 Bug Fix - bug fixed for the client

5. 🏠 Internal - Refactored codebase, adapter design pattern introduced, structural overhauls

Purpose

To introduce steady and improvised structural changes within the SDK to make future development much more simpler and easier to build on top of. It generalizes many principles that can and will most likely be reused over and over again as we progress through the requirements. As a demonstration of this implementation, the 2nd requirement has been built naively with a few polishes remaining ( paramater explanation ).

This PR and merge will hopefully make it much more easier to rapidly develop newer features and for newer use cases.

Why?

Structural changes and a consistent design pattern in the SDK is helpful as much of the operations will overlap not just in terms of what solutions they provide, but the way in which they are preprocessed, accessed, and built on top of as layer on layer is quite useful to understand.

This PR helps to develop that aspect by building the following layer of how the internal control flow looks like,

High Level Interface (mapillary.py)
--------------------------------------------
Controllers (business logic)
--------------------------------------------
Models (API/Adapters) -> Pipeling(Utils/filtering)
--------------------------------------------
Cient (Request/Response)
--------------------------------------------
API_URLS (/Config/API/)

The flow goes from top to bottom, and then from bottom to top.

As an example of how this looks like on the most important and developer side of the features, the controller layer, the code for the #13 simply boils down to,

def get_image_close_to_controller(
    longitude: float,
    latitude: float,
    kwargs: dict,
) -> dict:

    # Checking if a non valid key
    # has been passed to the function
    # If that is the case, throw an exception
    kwarg_check_for_images(kwargs)

    data = VectorTilesAdapter().fetch_layer(
        layer="image",
        zoom=kwargs["zoom"] if "zoom" in kwargs else 14,
        longitude=longitude,
        latitude=latitude,
    )

    # Filtering for the attributes obtained above
    if data["features"][0]["properties"] != {}:
        return pipeline(
            data=data,
            components=[
                {"filter": "coverage", "tile": kwargs["coverage"]}
                if "coverage" in kwargs
                else {},
                {"filter": "organization_id", "organization_ids": kwargs["org_id"]}
                if "org_id" in kwargs
                else {},
                {
                    "filter": "haversine_dist",
                    "radius": kwargs["radius"],
                    "coords": [longitude, latitude],
                }
                if "radius" in kwargs
                else 1000,
            ],
        )

Which has logic that can be explained very simply, looks very Pythonic to read, and is easy to replicate results and explain to other developers without worrying too much of internal details that are, and can be, most likely repeated again and again in multiple instances.

Feedback required over

Ffeedback required from this PR is over,

• A quick pair of :eyes: on the code
• Discussion on the technical approach
• Critique on design

Or something else entirely that you have in mind

Feedback required when

The next maintainer meeting would be fine, I think.

Mentions

• @cbeddow about the approach, the method, the naive implementation, the structure, and the interface so far
• @OmarMuhammedAli same as above

References (OPTIONAL)

This PR is connected, more or less, to the issues such as #13, #14, #26, #33, #35

• Mapillary API Documentation
• Getting Started with the new Mapillary API v4

[OmarMuhammedAli]

A lot to look at indeed! Although I'm sure this PR provides some great structure/feature changes, I believe it needs to be broken down into multiple PRs, for the sake of abiding by the single-responsibility principle for each PR. Also, the PR introduces multiple big changes that I think will be more practical if discussed individually and reviewed thoroughly in their own PRs. So closing this PR and opening new multiple, specialized ones is the way to go imo, then we can review them efficiently one by one!

[Rubix982]

@OmarMuhammedAli, I think it may be difficult to do it at this stage right now for a few reasons,

1. Rolling back all of the commits and file changes, and converting each file or set of file changes into its own branch to send as a PR
2. Separating the file changes just enough that they integrate well enough, at the same time, don't break anything, into their own PRs I thought was a bit cumbersome because they would be lacking complementary components to make complete sense of them
3. Having to go back at considering from the 1st commit how to separate the commits in even paces

I'll keep this in mind for my next PR, though.

What do you think?

[Rubix982]

@cbeddow requested changes made in the recent commit.

[cbeddow]

@OmarMuhammedAli @Rubix982 up to you two if you want to break this one down or just merge as is, let me know or merge on your own.

[OmarMuhammedAli]

@cbeddow Noted! @Rubix982 and I will have a 1:1 meeting later today or during the weekend to decide on the best approach for the situation.

[Rubix982]

Closing in favor of smaller PRs.