Csse layout input_data

[loriab]

Description

currently atop #458

Changelog description

• implement AtomicResult.input_data = AtomicInput in all the harnesses and adjust some output checks as needed. Check extras is getting separated btwn in/out as formerly merged.

Status

• Code base linted
• Ready to go

[codecov]

Codecov Report

Attention: Patch coverage is 96.22642% with 6 lines in your changes missing coverage. Please review.

Project coverage is 81.94%. Comparing base (0642655) to head (d22eb37).
Report is 8 commits behind head on next2024.

Additional details and impacted files

[loriab]

Requesting review @davidbrownell, @krachwal, @rfievet, @ketanbj, @varun646, @jyoung3131

[davidbrownell]

I don't have enough context to know if the following comment is helpful or falls into the category of over-architecture...

It seems that there is a repeating pattern at play of "if the request is v2, access a value in this way; if the request is v1, access a value in that way.". Perhaps there is a design that can group the v2 ways of accessing data and the v1 ways of accessing data. It might look something like...

from ABC import ABC, abstractmethod

class RequestAccessor(ABC):
    @staticmethod
    def GetAccessor(request):
        if "v2" in request.node.name:
            return V2RequestAccessor(request)
        
        return V1RequestAccessor(request)
        
    def __init__(self, request):
        self.request = request

    @abstractmethod
    def RequestDriver(self):
       raise NotImplementedError("Abstract method")

class V1RequestAccessor(RequestAccessor):
    def GetDriver(self):
        return self.request.driver

class V2RequestAccessor(RequestAccessor):
    def GetDriver(self):
        return self.request.input_data.driver

I'm wondering if this class might be valuable to people going through the v1 to v2 migration.

There may even be a simpler way to do this if the difference is always that v2 has input_data while v1 does not.

[loriab]

Thanks, but I think explicit is better than accessor in this case. I'm expecting that users going through the v1 to v2 migration who don't want to confront changes at first will take their existing input or output and run .convert_v(1|2) on it. Your route would let them deal with one field at a time, but locations may vary depending on what order one migrates the fields, so that's getting complicated. The clutter in the tests could be done away with by adding convert_v calls to the checkver_and_convert that are already set up in all the tests (in fact, that's what my work items were planning on before I decided on explicit logic). But since the tests are the most abundant source of model schema for users, I thought explicit might be better. It doesn't help the user, but typing the logic for me isn't as much of a burden as it looks (see next PR in series) since CoPilot suggests with excellent accuracy.