I want to get Knut's opinion about the filtering part.

@Midtveit any thoughts?

I've tested the stories a bit, and it looks really nice! Some thoughts

• @Midtveit has some thoughts about the perforations design and is requesting more control of the appearance (eg. perforation ranges, colors for open/closed, etc). But this is a good first step.
• Re. the above point, the API of the wells layer becomes cluttered with so many options, especially if we want more control over the appearance of perforations, etc. I think it's more future proof to nest related props into single objects in order to avoid cluttering and to make the API easier to understand. I also understand the downsides, but my personal preference is to nest related properties.
• In the formations example, it would be nice to be able to optionally show the formation logs (just the story, not built in to the layer)

Many comments are given by VS code locally ;)
I not quite sure what you mean, @w1nklr ? Do you mean that these things been showing up as errors/warnings in your VS Code? None of these have been flagged on my end, which means we might have inconsistent editor settings. Shouldn't the linter configs ensure this doesn't happen?

Many comments are given by VS code locally ;)
I not quite sure what you mean, @w1nklr ? Do you mean that these things been showing up as errors/warnings in your VS Code? None of these have been flagged on my end, which means we might have inconsistent editor settings. Shouldn't the linter configs ensure this doesn't happen?

Yes, the proposed changes in this review are (mostly) proposed by my VS code.
Some are coming from the linter, but others are from sonarqube (which is setup for our internal projects, but is also applied for whatever gets open with VS code).

Definitively, we should setup formal lint rules to add some consistency to our code (like enforcing naming rules, ordering imports, avoiding use of any...).

An example of such a change proposal:

Right, @w1nklr I resolved most of them, although there were some changes I'm not fully convinced are better... Are the remaining issues things that are being flagged, if so, I'll go along with them to make things easier; but we should make sure we sync up our linting rules soon to avoid that in the future

I've tested the stories a bit, and it looks really nice! Some thoughts

• Midtveit has some thoughts about the perforations design and is requesting more control of the appearance (eg. perforation ranges, colors for open/closed, etc). But this is a good first step.
• Re. the above point, the API of the wells layer becomes cluttered with so many options, especially if we want more control over the appearance of perforations, etc. I think it's more future proof to nest related props into single objects in order to avoid cluttering and to make the API easier to understand. I also understand the downsides, but my personal preference is to nest related properties.
• In the formations example, it would be nice to be able to optionally show the formation logs (just the story, not built in to the layer)

@hkfb

• Is it fine to leave it as is for this PR and open an issue to discuss future improvements? I did create a getMarkerColor accessor in the sublayer, I could expose that to atleast allow open/closed colors
• Thats a fair point, especially when dealing with larger sub-layers like this. I'll change it to a nested object
• Did you mean that I should show a log viewer track on the side, like this?
  

Does this seem okay, @hkfb ? Technically the dashed sections and screen markers are two different layers, but I think its fair to put their settings together.

/* --- Screens and perforations --- --- --- --- ---*/
    markers?: {
        /** Enables simple line markers at the start and end of screen sections.
         *
         * **Note:** These markers are currently only designed for 2D, so they are not guaranteed to look nice in 3D
         */
        showScreens?: boolean;
        /**
         * Renders screen sections of the trajectory as dashed segments.
         *
         * **Note: If enabled, the COLORS sub-layer will be replaced by the SCREEN_TRAJECTORY sub-layer**
         */
        showScreenTrajectoryAsDash?: boolean;
        /** Specifies the dash factor for screen sections. */
        getScreenDashFactor?: DashAccessor;

        /** Enables visualization of trajectory perforations.
         *
         * **Note:** These markers are currently only designed for 2D, so they are not guaranteed to look nice in 3D
         */
        showPerforations?: boolean;

        /** Specifies colors of markers at a per-marker basis */
        getMarkerColor?: FlatWellMarkersLayerProps<WellFeature>["getMarkerColor"];
    };

Should we also create an object for the filter? I was thinking that we could consider keeping that flat since it very directly affects the top-level rendering of the entire well-layer, as opposed to labels, markers and such which are just a specific sublayer

I've tested the stories a bit, and it looks really nice! Some thoughts

• Midtveit has some thoughts about the perforations design and is requesting more control of the appearance (eg. perforation ranges, colors for open/closed, etc). But this is a good first step.
• Re. the above point, the API of the wells layer becomes cluttered with so many options, especially if we want more control over the appearance of perforations, etc. I think it's more future proof to nest related props into single objects in order to avoid cluttering and to make the API easier to understand. I also understand the downsides, but my personal preference is to nest related properties.
• In the formations example, it would be nice to be able to optionally show the formation logs (just the story, not built in to the layer)

@hkfb

• Is it fine to leave it as is for this PR and open an issue to discuss future improvements? I did create a getMarkerColor accessor in the sublayer, I could expose that to atleast allow open/closed colors

I'm fine with that as long as it'll be easy to add later.

• Thats a fair point, especially when dealing with larger sub-layers like this. I'll change it to a nested object
• Did you mean that I should show a log viewer track on the side, like this?
  

That's interesting, but I meant (optionally) showing the logs along the trajectory. It can help understand and QC the filtered formations.

Does this seem okay, @hkfb ? Technically the dashed sections and screen markers are two different layers, but I think its fair to put their settings together.

/* --- Screens and perforations --- --- --- --- ---*/
    markers?: {
        /** Enables simple line markers at the start and end of screen sections.
         *
         * **Note:** These markers are currently only designed for 2D, so they are not guaranteed to look nice in 3D
         */
        showScreens?: boolean;
        /**
         * Renders screen sections of the trajectory as dashed segments.
         *
         * **Note: If enabled, the COLORS sub-layer will be replaced by the SCREEN_TRAJECTORY sub-layer**
         */
        showScreenTrajectoryAsDash?: boolean;
        /** Specifies the dash factor for screen sections. */
        getScreenDashFactor?: DashAccessor;

        /** Enables visualization of trajectory perforations.
         *
         * **Note:** These markers are currently only designed for 2D, so they are not guaranteed to look nice in 3D
         */
        showPerforations?: boolean;

        /** Specifies colors of markers at a per-marker basis */
        getMarkerColor?: FlatWellMarkersLayerProps<WellFeature>["getMarkerColor"];
    };

Should we also create an object for the filter? I was thinking that we could consider keeping that flat since it very directly affects the top-level rendering of the entire well-layer, as opposed to labels, markers and such which are just a specific sublayer

Ok, I'm good with this.

Tiny remark, I suggest using tsdoc syntax, eg. @remarks instead of "Note".

I've tested the stories a bit, and it looks really nice! Some thoughts

* @Midtveit has some thoughts about the perforations design and is requesting more control of the appearance (eg. perforation ranges, colors for open/closed, etc). But this is a good first step.

I spoke with Knut yesterday.

The main point is that perforations are usually on an MD range and not at a single MD.
This means that PerforationProperties should have mdStart: number; and mdEnd: number; fields instead of single md.

If it's too much change for this PR, you can create a follow-up PR.

BTW, I'm also wondering if:

• PerforationProperties.status should not be an string union for stronger typing (like "shot" | "closed" or "open" | "closed")
• what is PerforationProperties.mode ?
• FormationProperties could have same md names than ScreenProperties : mdStart and number;

These types should have a documentation also....

The last stuff should now have been fixed, although I left the perforation issue as is. Im unfamiliar with the domain specifics for this, so it probably be better if one of you do a the more specific changes regarding that in a new PR; we have another project that is waiting for this functionality, and as this PR covers our usecase, so it' be nice to resolve this PR if possible.

As for the specific datatypes for sceens and perforations, the only thing we need is the dates and md points, so feel free to change those to whatever is needed

Made an issue to track the remaining issues and other things #2705