Shader UV/Texture mapping calculated wrong when using regions (using full texture size)

Godot version
3.4.2
System information
PopOS
Issue description
Simple shader that transforms a sprite using a gradient texture. Three images:

A single image 80x64, this is the image on the right and works, it opens/closes like the gradient you see
A sprite 240x128 with 8 images but a region specifying a single sprite, this is the image in the middle
A sprite defined exactly as (2) but as a .tres file from a larger sprite.

Open the node file if not open, select any image (I have shared resources), go to the material and slide the dissolve value
Bug 1: The texture/UV calculations seem to be taking into the account the entire size of the source image, so if you look image 3 (the single png) works correctly matching the texture, but image 1 and 2 do not. If you look closely the second image seems to be using half the gradient because it is twice the size of the image, but finishes at the same time.
Bug 2: I don't know if this is just the size or that one is a tres file, but while images 2 and 3 finish at the same time, the .tres file finishes rapidly and cannot be revealed smoothly.
https://user-images.githubusercontent.com/12863685/151679649-b5961226-057b-4f60-a2e1-4f1572c73559.mp4
If these are known bugs or valid bugs, I'd appreciate a workaround if any.
Steps to reproduce
As description.
Minimal reproduction project
tres_shader_fail.zip

Just a note, that weird colouring you can see (my bug 3) does not happen with the default icon.png. But this is just an ordinary RGBA8 png?

Hello again,
I think something is very, very wrong with 3.4.2 and it looks like it is related to the import process, regarding my bug 3, when using any shader:
This is the normal image, it is using the 2d Pixel template (i.e. filter is off).

When I turn on my shader, any shader, which literally does nothing:
COLOR=texture(TEXTURE,UV);
This is what it looks like:

If I then disable fix_alpha_border, it looks like this:

And with fix_alpha_border off as above, it does this to the textures that atlases:

Can you reproduce this after enabling Force Png in the project settings, exiting the editor, removing the .import/ folder within the project then opening the editor again?

I'll try now... hang on, but have you tried running the project?

With 3.5.1-stable, today I was trying to apply a UV box mask via a canvas_item custom shader to a Sprite with region and I came across this same bug.
Another repro:

Set up a Sprite region from an atlas texture, e.g. isolate an object from any of Kenny's sprite atlases.
Swap in a custom shader.
Use this shader code:

shader_type canvas_item;
void fragment() {
	COLOR = texture(TEXTURE, UV * vec2(1.0));
}

Observe that the correctly regioned sprite is displaying.

Change the 1.0 to 1.2, i.e. zoom out 20%.
Observe that there are other atlas tiles now showing in the region.  Godot is using the entire atlas UV space.

So I guess this means you cannot do UV manipulation in custom shaders when using a Sprite with a defined region.

I also needed to apply a relative effect to my sprite (pixelate effect).
But it's not a bug, the UV currently received refer to the underlying texture.
So we have to get somehow the information of the currently set region to be able to make the necessary transformations to get "local" region_UV.
Here are two functions that allow to do this transformation:
vec2 uv_to_region_uv(vec2 uv, vec2 texture_pixel_size) {
	return (uv-(rect_info.xy*texture_pixel_size))/(rect_info.zw*texture_pixel_size);
}

vec2 region_uv_to_uv(vec2 region_uv, vec2 texture_pixel_size) {
	return (rect_info.xy*texture_pixel_size)+region_uv*(rect_info.zw*texture_pixel_size);
}

void fragment() {
	vec2 region_uv = uv_to_region_uv(UV, TEXTURE_PIXEL_SIZE);
	// Any local uv transformation
	vec2 pixelated_region_uv = pixalate(region_uv, rect_info.zw)+TEXTURE_PIXEL_SIZE/2.;
	// Convert back to UV texture to sample it
	vec2 texture_uv = region_uv_to_uv(pixelated_region_uv, TEXTURE_PIXEL_SIZE);
	COLOR = texture(TEXTURE, texture_uv);
}

Here, rect_info contains the position and size info of the currently set region.
Here I set manually the uniform rect_info, but I had the same problem in Unity and I made a dirty script that allows to update the uniform automatically.
using System;

using UnityEngine;

[ExecuteInEditMode, RequireComponent(typeof(SpriteRenderer))]
public class SpriteRectUniformProvider : MonoBehaviour
{
    // (...)
    public void UpdateMaterialPropertyBlock() {
        Sprite sprite = _spriteRenderer.sprite;
        if (sprite != null)
        {
            MaterialPropertyBlock mpb = new MaterialPropertyBlock();
            _spriteRenderer.GetPropertyBlock(mpb);

            Vector4 spriteRect = new Vector4(
                sprite.rect.xMin / sprite.texture.width,
                sprite.rect.yMin / sprite.texture.height,
                sprite.rect.width,
                sprite.rect.height);

            mpb.SetVector("_spriteRect", spriteRect);
            _spriteRenderer.SetPropertyBlock(mpb);
        }
        else
        {
            string logMessage = $"[{GetType().Name}] Unable to update uniform as the sprite isn't set yet on the SpriteRenderer";
            if (Application.isPlaying)
            {
                JdLog.LogThat("", LogLevel.LevelInfo, logMessage);
            }
            else
            {
                Debug.Log(logMessage);
            }
        }
    }
}

I don't know if it's useful to have the UV of the underlying texture (when using a region) but if it's the case (so you can't just change the UV), a great idea would be to have a uniform build-in variable to get the region info if needed.
I started looking into the engine (open source yeah), I would like to make a PR to bring this functionality. Do you think this is the right direction (before I start anything), or do you have any other ideas for a solution in this situation ?

@Calinou do you have an opinion ?
I search in those files :

godot/servers/rendering/shader_language.cpp
godot/servers/rendering/shader_compiler.cpp
Do you have some tips to allow me to implement built-in new uniform (region_rect) for the canvas_item shader_type ?

I believe I'm currently experiencing this same issue in 4.1 and it's quite the burden. Is there any decently viable workaround? Can this even be fixed someday?

Hello,
@Mickeon, you can use my workaround using unction uv_to_region_uv and region_uv_to_uv above.
I proposed to fix it myself (or make a uniform build-in) to manage this case, but I didn't get any feedback from the godot team before starting a PR and working on it.
@Calinou ?

@dsmtE Feel free to open a pull request for this :slightly_smiling_face:

I would thank you for a PR as this is still present in 4.2.1-stable 🙂

Hello @Calinou,
I'm starting to work on it, it's a bit difficult to understand and integrate into the code base. First, I'll try to make it work in the dirtiest way and then we'll rework it and do it the right way.
I found the glsl canvas_item shader (./servers/rendering/renderer_rd/shaders/canvas.glsl) and tried to figure out where the sprite region information (offset and size) came from.
Here's a list of several parts of the code that I found interesting:

./servers/rendering/renderer_rd/renderer_canvas_render_rd.cpp(method "RendererCanvasRenderRD::_render_item", line 508)
./scene/2d/sprite_2d.cpp(method "Sprite2D::set_region_enabled", line 212)
.//scene/resources/texture.cpp(method "Texture2D::draw_rect_region", line 77)

I managed to add a custom variable to make certain information accessible from my Godot shader (draw_data.src_rect and draw_data.dst_rect)
vec4 region_position_and_size = draw_data.dst_rect;

According to my tests, raw_data.dst_rect.zw seems to match the region size.
And so I'm able to perform the desired scale transformation on my UVs when the region has an offset of 0.
But, I can't figure out how to get the position offset. I can't figure out what raw_data.dst_rect.xy is.
Can you enlighten me a little or ask someone who has information on this subject so that I can continue?
Thanks in advance 🙂

Hello @Calinou, I'm starting to work on it, it's a bit difficult to understand and integrate into the code base. First, I'll try to make it work in the dirtiest way and then we'll rework it and do it the right way.
I found the glsl canvas_item shader (./servers/rendering/renderer_rd/shaders/canvas.glsl) and tried to figure out where the sprite region information (offset and size) came from.
Here's a list of several parts of the code that I found interesting:

./servers/rendering/renderer_rd/renderer_canvas_render_rd.cpp(method "RendererCanvasRenderRD::_render_item", line 508)
./scene/2d/sprite_2d.cpp(method "Sprite2D::set_region_enabled", line 212)
.//scene/resources/texture.cpp(method "Texture2D::draw_rect_region", line 77)

I managed to add a custom variable to make certain information accessible from my Godot shader (draw_data.src_rect and draw_data.dst_rect)
vec4 region_position_and_size = draw_data.dst_rect;

According to my tests, raw_data.dst_rect.zw seems to match the region size. And so I'm able to perform the desired scale transformation on my UVs when the region has an offset of 0.
But, I can't figure out how to get the position offset. I can't figure out what raw_data.dst_rect.xy is. Can you enlighten me a little or ask someone who has information on this subject so that I can continue?
Thanks in advance 🙂

Is it possible to share your custom build of the Engine? Would love to help.

Hello,
Thanks for your help @Delsin-Yu!
I have paused my work on this for a few weeks (for personal reasons).
This week, with the help of this message by @kleonc (on the Godot contributor chat), I've figured out how to use draw_data.src_rect for my needs.
I still don't understand all the shader variants and defines (USE_PRIMITIVE, USE_ATTRIBUTES, USE_NINEPATCH), I would be happy to discuss it with you if you can explain it to me (anyway there will be reviews and exchanges after my first "fix" proposal because I didn't manage the gles3 part for the moment to make my life easier).
I will iterate a bit more this week and come back with some proposals and examples here on the possibilities of how this fix should be properly integrated into the engine (in a backward-compatible way or not, change the management of the existing UV variable or just make available new built-in functions and variables). Furthermore, I will also give you a link to the branch I'm working on at the same time.

Hello,
I have a first working draft :confetti_ball: !
Basically, I added two new built-in variables and two new  built-in functions:

REGION_SIZE : (vec2) which contains the size of the region set in the inspector (in pixels)
REGION_POSITION :  (vec2) which contains the position of the region (in pixels)
region_to_texture_uv(vec2 uv): that take uv (from range [0, 1] and transform them to scan and shift to the region location

vec2 region_to_texture_uv(vec2 uv) {
    return uv * region_info.zw + region_info.xy;
}

texture_to_region_uv(vec2 uv): that do the reverse transformation to get normalized [0,1] uv coordinates from the applied region uv

vec2 texture_to_region_uv(vec2 uv) {
    return (uv - region_info.xy) / region_info.zw;
}

These built-in allows us to do local transformation on the region sub-texture.
Here is an example of shader using these built-in to zoom in :
shader_type canvas_item;

void fragment() {
    // Convert texture space uv to region
    vec2 region_uv = texture_to_region_uv(UV);

    // Any local uv transformation : exemple with centered zoom effect
    float zoom_factor = 1.2;
    vec2 zoomed_uv = (region_uv-0.5)/zoom_factor+0.5;

    // Convert back to texture uv to be sampled properly using the texture(TEXTURE, uv) function
    vec2 texture_uv = region_to_texture_uv(zoomed_uv);

    // Sample the texture using right region UV after local transformation
    COLOR = texture(TEXTURE, texture_uv);
}

Doing it this way do not break any older shader (as the built-in UV variable isn't changed).
Another benefit of doing it that way is to be able to manipulate also the texture UV after the "back conversion" from local to texture uv.
Here an example of shifting the UV by the size of the region (could be useful for sprite Sheet effect when the sprite2D use region already):
shader_type canvas_item;

void fragment() {
    vec2 texture_uv = UV;

    // Shift the UV on the x axis using REGION_SIZE to obtain the new part of the texture
    // with the same size as the region (next texture in a horizontal sheet sprite, for example)

    // This must be multiplied by TEXTURE_PIXEL_SIZE
    //because REGION_SIZE contains the size of the region in pixels (16x16 for example) and to offset the uvs we need to be in normalized uv space
    texture_uv.x += REGION_SIZE.x*TEXTURE_PIXEL_SIZE.x;
    COLOR = texture(TEXTURE, texture_uv);
}

What do you think of this proposal (from a technical and user point of view)?
@Delsin-Yu, @belzecue, @Mickeon, @chucklepie, @GrauBlitz

I prefer this approach because it doesn't break backwards compatibility, but allows you to do whatever you want in custom shaders with regions.
However, I wonder if we shouldn't change what the UV variable itself contains. After all, the basic behavior of regions (i.e. texture UVs for sampling and not local UVs [0,1] for the region before sampling) may not be the most intuitive in consideration of the preceding messages.
We could then imagine a possible solution that :

change the content of UV to contain local UV instead (equivalent to region_uv in my previous example)
propose a function for sampling in a texture and applying the UV change for the region at that moment, or expose the region_to_texture_uv function but make it necessary before sampling to display what you want.

An example of simple fragment shader become:
shader_type canvas_item;

void fragment() {
    COLOR = texture(TEXTURE, region_to_texture_uv(UV));
}

and with zoom effect:
shader_type canvas_item;

void fragment() {
    vec2 zoomed_uv = (UV-0.5)/zoom_factor+0.5;
    COLOR = texture(TEXTURE, region_to_texture_uv(zoomed_uv));
}

This solution breaks backward compatibility, as it requires the transformation function to be called before sampling.
An even "smoother" solution would be to change the behavior of the texture(TEXTURE, UV) sampling function itself to perform the region transformation implicitly.
Example:
shader_type canvas_item;

void fragment() {
    // implicit region UV transformation here
    COLOR = texture(TEXTURE, UV * 1.2);
}

But this would prevent the actual texture UVs from being modified as freely as we want, as in my previous example of shifting region effect, where the shift must be performed on the texture UVs and not on the UVs local to the region (so after the potential implicit region_to_texture_uv transformation).
And I don't know how to do it technically either at the moment, another reason not to go for this solution. :smiling_face_with_tear:

Here my repo and branch I'm working on: https://github.com/dsmtE/godot/tree/dev/region_texture_mapping
Reminder, this is a rough version and doesn't support gles3 and probably other things to be integrated properly into the engine.  I prefer to discuss it and get your feedbacks before continuing my work.

@dsmtE Wow, that's a good job you have done, congrats!
I think you can maybe even create a draft PR for this.
I prefer the REGION_SIZE/REGION_POSITION+region_to_texture_uv+texture_to_region_uv approach as well since it gives full control to the developer (not to mention it doesn't impact existing shader code, so it's not that hard to get merged).
I also like the idea of silently changing the texture(TEXTURE, UV) behavior so it kind of silently addresses the issue for future developers. But this breaks existing shaders that use uniform region size/region position to perform manual correction.

@dsmtE I would suggest directly opening a PR for this instead of talking about it here, to make it more focused. I have no opinions right now.

Glad to see if built-in variables or functions could be exposed, as it will be much more convenient to use~
Before a new PR got merged, this following piece of shader may be a bit helpful if someone is looking for a quick fix. It might be kind of dirty and not optimized, but it just works.
shader_type canvas_item;

const vec2 VERTICES[4] = vec2[](vec2(0.0),vec2(0.0,1.0),vec2(1.0),vec2(1.0,0.0));
varying vec2 originUV;
varying flat vec4 regionInfo;

void vertex() {
	originUV = VERTICES[VERTEX_ID];
	vec2 ouv = VERTICES[0];
	regionInfo.xy = VERTEX.xy/(ouv - 0.5);
	regionInfo.zw = UV;
}

vec2 region_from_texture(vec2 uv, vec2 size) {
	return  regionInfo.zw + uv / size * regionInfo.xy;
}

vec2 texture_from_region(vec2 uv, vec2 size) {
	return uv * size / regionInfo.xy - regionInfo.zw;
}

void fragment() {
	vec2 uv_to_transform = (originUV - 0.5) * sin(TIME) * 3.0 + 0.5;

	const vec2 size = vec2(textureSize(TEXTURE, 0));
	vec2 restoredUV = region_from_texture(uv_to_transform,size);

	COLOR = texture(TEXTURE, restoredUV);
}

https://github.com/godotengine/godot/assets/33864304/3c26d862-3a6e-43be-8357-d1a4c96a121a

@dsmtE AFAICT the rendering team don't like to break previously working shaders, so I'd guess that auto-changing UV to mean different thing (depending on the context) than it currently does is a no-no. Same about changing the behavior of already existing texture function. So I suggest focusing on non-compat-breaking solutions.
What could probably be considered as a viable solution for changing what the built-in UV refers to is adding a new render mode. Just as currently e.g. world_vertex_coords can be used to change what VERTEX within vertex() is, something like region_uv_coords could maybe be added for changing what UV refers to? :thinking:
But regardless, I'd say instead of commenting here you should probably open a proposal in https://github.com/godotengine/godot-proposals, that's exactly what that repo is for - there's some issue and you're proposing a specific solution(s) for it. Then it can be discussed in detail in there (technical details, different approaches, etc.).

Thanks for all your answers, I just open the proposal to discuss it, and after, I will continue to work on my PR.


Use ahash in cost tracker

Problem
cost_tracker uses HashMap to keep tracking writable accounts CUs. HashMap isn't fast.
Summary of Changes

Add benchmarking test for cost_tracker
Use ahash for better performance

Fixes #

Master:
test bench_cost_tracker_contentious_transaction     ... bench:  11,625,587 ns/iter (+/- 254,789)
test bench_cost_tracker_non_contentious_transaction ... bench:  17,255,980 ns/iter (+/- 391,458)

This PR:
test bench_cost_tracker_contentious_transaction     ... bench:   3,587,329 ns/iter (+/- 85,738)
test bench_cost_tracker_non_contentious_transaction ... bench:   6,096,914 ns/iter (+/- 163,584)

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.1%. Comparing base (aa2f078) to head (231e2f5).
Report is 44 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master    #1252     +/-   ##
=========================================
- Coverage    82.1%    82.1%   -0.1%
=========================================
  Files         886      886
  Lines      236417   236424      +7
=========================================
- Hits       194266   194253     -13
- Misses      42151    42171     +20


Email Template Manager Code&Functionalty Review

[ ] List grid
[ ] Select2
[ ] Lang connect to journal languages


Support mouse events.

Adds mouse events support.

Thanks! Sorry it took so long to get back to this.


Android Build failing with 'cannot find symbol call.setKeepAlive(true)'

I've built a simple Ionic App with Capacitor v3.
When trying to build this app with android studio, the build is failing with the following error:
@capacitor-community\background-geolocation\android\src\main\java\com\equimaps\capacitor_background_geolocation\BackgroundGeolocation.java:53: error: cannot find symbol
        call.setKeepAlive(true);
            ^
  symbol:   method setKeepAlive(boolean)
  location: variable call of type PluginCall

That's weird, Capacitor v3 definitely defines a method setKeepAlive on the type PluginCall, see source.
Is the method present in the PluginCall.java file in your project?

It seems like I did something wrong while setting up the project / Capacitor v3.
I created a new project from scratch and the error does not appear anymore.


Refactor Chat Interface and Introduce ChatListHeader Component

closes #87

Introduced ChatListHeader Component
Integrated ChatCard with ChatListHeader
Updated Styling
Refactored chat.vue

I hope this is similar to what you had in mind, I am all ears for changes and suggestions

Thank you for the PR, I'll review it in the following weekday.


Error in running flask server

WinErrno[ 10061] No connection could be made because the target machine actively refused it

This is a windows error. Click here for more information.


Event delegation sugar for the mediator pattern

When using the mediator pattern, often the parent is listening to an event of a child and perform operations on a different child. For example implementing a refresh-button with paper-button and iron-ajax:
<template>
  <iron-ajax
    id="ajax"
    ....
    ></iron-ajax>
  <paper-button on-tap="refreshClick">Refresh</paper-button>
</template>
<script>
Polymer({
  is: 'refresh-button',
  refreshClick: function() {
    this.$.ajax.generateRequest();
  }
});
</script>

This pattern requires additional methods in the parent to just infer a method on a child. It would be nice to have some sugar that allows direct references to a child method. E.g.:
<template>
  <iron-ajax
    id="ajax"
    ....
    ></iron-ajax>
  <paper-button on-tap="ajax.generateRequest">Refresh</paper-button>
</template>
<script>
Polymer({
  is: 'refresh-button'
});
</script>

This sugar allows the interaction between children via the mediator pattern, while preventing countless of methods to be defined in the parent.

You can implement this yourself by overriding _parseTemplateNodeAttribute


Make Atomist web hook configurable

Currently the atomist web hook url is hard coded wherever it is used. Since this will change depending on the instance we are using this needs to be extracted to a configurable property.

Please use Type-safe Configuration Properties for this.
See this for context reference.


fix(api): pick_up_tip in 2.14+ protocols does not pick up tips before starting tip

Overview
Closes RESC-145.
In api version 2.14 and above (which uses the Protocol Engine), a regression was introduced that made it that if you chose a starting tip and used pick_up_tip without any arguments, the wells after the starting tip would be A1, B1, C1, etc instead of returning the wells that followed the starting one. This has been addressed so that the pick up tip behavior is identical to versions 2.13 and below.
Test Plan
Tested the following protocol with both API levels 2.13 and 2.14 on an OT-2 to verify that they picked up tips in B2, C2, D2...A3
metadata = {
    'protocolName': 'Tip Regression Behavior',
}

requirements = {
    "robotType": "OT-2",
    "apiLevel": "2.13"
}

def run(context):
    tip_rack = context.load_labware("opentrons_96_tiprack_20ul", location=5)
    pipette = context.load_instrument("p20_single_gen2", mount="left", tip_racks=[tip_rack])

    pipette.starting_tip = tip_rack["B2"]

    for _ in range(8):
        pipette.pick_up_tip()
        pipette.drop_tip()

Changelog

fixed TipView.get_next_tip to drop potential wells before the starting tip well

Review requests
Risk assessment
Low, behavior only changes if starting tip is used, and that has been tested to ensure it matches with previous behavior.

@DerekMaggio would you please look at the failing G-code test?  I think this is correctly identifying a change in the G-Code capture due to a software change.

@y3rsh, I agree it is.
I will update the file and push it to this branch

Can this PR also fix up the docstring for InstrumentContext.starting_tip? 🫣
https://github.com/Opentrons/opentrons/blob/6ae02e467f59cd9e567caf5dd65ae210f3da2057/api/src/opentrons/protocol_api/instrument_context.py#L104
Something like
Which well of a tip rack the pipette should start at when tracking tips. The first call to
:py:meth:`.pick_up_tip` with no argument will pick up from that well. Subsequent calls
will follow the ordering specified by the labware definition and :py:meth:`.Labware.wells`.

Now in edge, this Known Limitation warning should be changed to a Bug Fix when this is implemented.
https://github.com/Opentrons/opentrons/blob/a639bae50dc1f5974c4070af32295dc81ae64a20/api/docs/v2/versioning.rst?plain=1#L163-L165

There was a question of whether we should:

Just fix the bug. This would fix it retroactively for PAPIv2.14. So certain Python files would behave differently on robot software v7.0 than on v6.3.
Preserve the buggy behavior in PAPIv2.14 but fix it in PAPIv2.15, by checking the protocol's apiLevel.

@sanni-t, @jbleon95, @laviera and I discussed this. We're going with just fixing the bug.
There are certain cases where, given our current architecture, it's impractical to preserve bug-for-bug compatibility. This is one of those. @sanni-t brought up a prior case related to transfer planning. I brought up the whole Magnetic Module GEN1 engage height thing, where our attempts to preserve bug-compatibility have backfired and caused lasting confusion and additional bugs years later.
There are architectural things that we can do to improve this, which I have been meaning to write up, but they would be a long-term post-Flex thing.


Strings in coordinates may be truncated when saving concatenated rasters to zarr

What happened?
I would like to concatenate two dataarrays or two datasets, save them to zarr, read them into xarray again at a later time, and have all the coordinates intact.
However, if I concatenate two dataarrays or datasets along a string-based coordinate, if the first dataarray in the concatenation has a coordinate with shorter maximum string length than the second dataarray, saving the concatenated dataset to zarr can truncate the string.
This problem only arises when:

First dataarray's string coordinate has an encoding["dtype"] attribute
First dataarray's string coordinate length is shorter than the second dataarray's.

What did you expect to happen?
I expect the xarray dataset that was read from the zarr to have the string coordinate the same as the input dataset that I wrote in, with no truncation.
Minimal Complete Verifiable Example
import logging
import tempfile
from pathlib import Path

import dask.array as da
import numpy as np
import xarray as xr

logger = logging.getLogger(__name__)

def mb_raster_with_custom_band_names(bands: list[str]) -> xr.DataArray:
    """
    Generates a multi-band raster with custom band names.

    This function creates a multi-band raster with a specified list of band names. The raster is generated with random
    data and has a shape of (len(bands), 100, 100). The raster is assigned the EPSG:32733 coordinate reference system.

    :param bands: A list of strings specifying the band names for the raster.
    :return: A multi-band raster with custom band names as an xarray DataArray.
    """
    n_x = 100
    n_y = 100

    data_with_custom_band_names = xr.DataArray(
        (da.random.random_sample(size=(len(bands), n_x, n_y)) * 255).astype("float32"),
        coords={"band": bands, "y": np.arange(n_y), "x": np.arange(n_x)},
    )
    return data_with_custom_band_names

def zarr_round_trip(raster: xr.DataArray | xr.Dataset) -> xr.DataArray:
    """Simple function to test the round trip of a raster to zarr and back."""
    with tempfile.TemporaryDirectory() as tmpdirname:
        tmp_zarr_path = Path(tmpdirname) / "tmp_zarr.zarr"
        raster.to_zarr(tmp_zarr_path, mode="w")

        round_trip_raster = xr.open_dataset(tmp_zarr_path, engine="zarr")

    return round_trip_raster

if __name__ == "__main__":
    logging.basicConfig(level=logging.INFO)

    # Make a multi-band raster with short band names.
    rgb_raster = mb_raster_with_custom_band_names(["blue", "green", "red"])
    # Force the band encoding dtype to be equal to the band value dtype. This is what was triggering the band name
    # truncation issue when saving to zarr.
    rgb_raster.band.encoding["dtype"] = rgb_raster.band.dtype

    # Make another multi-band raster with long band names.
    long_rgb_raster = mb_raster_with_custom_band_names(["the_sky_is_blue", "the_grass_is_green", "the_rose_is_red"])
    long_rgb_raster.band.encoding["dtype"] = long_rgb_raster.band.dtype

    # concat two rasters with xr_concat (short band name raster first)
    concat_raster_short_first = xr.concat([rgb_raster, long_rgb_raster], dim="band")

    logger.info(f"{concat_raster_short_first.band.values=}")
    # array(['blue', 'green', 'red', 'the_sky_is_blue', 'the_grass_is_green', 'the_rose_is_red'], dtype='<U18')

    logger.info(f"{concat_raster_short_first.band.dtype=}")
    # dtype('<U18')

    logger.info(f"{concat_raster_short_first.band.encoding['dtype']=}")
    # dtype('<U5')

    concat_raster_short_first_reread = zarr_round_trip(concat_raster_short_first)
    logger.info(f"{concat_raster_short_first_reread.band.values=}")
    # array(['blue', 'green', 'red', 'the_s', 'the_g', 'the_r'], dtype='<U5')

    logger.info(f"{concat_raster_short_first_reread.band.dtype=}")
    # dtype('<U5')

    logger.info(f"{concat_raster_short_first_reread.band.encoding['dtype']=}")
    # dtype('<U5')

MVCE confirmation

[X] Minimal example — the example is as focused as reasonably possible to demonstrate the underlying issue in xarray.
[X] Complete example — the example is self-contained, including all data and the text of any traceback.
[X] Verifiable example — the example copy & pastes into an IPython prompt or Binder notebook, returning the result.
[X] New issue — a search of GitHub Issues suggests this is not a duplicate.
[X] Recent environment — the issue occurs with the latest version of xarray and its dependencies.

Relevant log output
INFO:__main__:concat_raster_short_first.band.values=array(['blue', 'green', 'red', 'the_sky_is_blue', 'the_grass_is_green',
       'the_rose_is_red'], dtype='<U18')
INFO:__main__:concat_raster_short_first.band.dtype=dtype('<U18')
INFO:__main__:concat_raster_short_first.band.encoding['dtype']=dtype('<U5')
INFO:__main__:concat_raster_short_first_reread.band.values=array(['blue', 'green', 'red', 'the_s', 'the_g', 'the_r'], dtype='<U5')
INFO:__main__:concat_raster_short_first_reread.band.dtype=dtype('<U5')
INFO:__main__:concat_raster_short_first_reread.band.encoding['dtype']=dtype('<U5')

Anything else we need to know?
The order of the dataarray in xr.concat mattered. If you run xr.concat([rgb_raster, long_rgb_raster], dim="band"), the truncation will happen; but if you run xr.concat([long_rgb_raster, rgb_raster], dim="band") , it will not.
I attempted to unwrap the call chain when we save a dataset or dataarray to zarr:

In the xarray.DataArray.to_zarr() function, xarray.DataArray is implicitly converted to xarray.Dataset before we save it to zarr: code.
Because we’re calling xarray.DataArray.to_dataset() directly with no additional argument, it calls xarray.DataArray._to_dataset_whole().
In xarray.DataArray._to_dataset_whole(), the coordinates attribute (a private attr called ._coords) of the dataarray is implicitly copied to the variable of the dataset. So whatever was in our dataset variables, it came from the ._coords private attr of the datarray.
The next clue as to how the dataset variables get saved to zarr is in xarray.backends.ZarrStore. Specifically, the dataset variable go through this encode_zarr_variable function. That function in turn calls xarray.conventions.encode_cf_variable. In there, the function just uses whatever the variable encoding was in the dataset, which came from the coordinate encoding from the data array.

Looks like a similar issue was handled in 2014 (https://github.com/pydata/xarray/issues/217); this bug appears to a corner case of when the encoding attr of a particular coordinate was left intact during concatenation.

A workaround would be .drop_encoding(). (When I'm using xarray for my own work, I generally always do this, but not sure that's necessarily correct...)
@pydata/xarray do we have a view / plan for how to handle encoding? We've discussed removing it in the past. I think it's one of those issues that experienced people can get around, but trip up new users, which means it's generally underweight in how core team & contributors (who are more experienced than average) prioritize issues.
Is there a case for defaulting to dropping encoding by default on read?

I've found that this bug also exists when using to_netcdf using the h5netcdf and netcdf4 engines. Using .drop_encoding before writing the file to disk also resolves the issue for me.

This issue is also present when using to_netcdf using the netcdf4 and h5netcdf engines.
@max-sixty's suggestion to drop_encoding() before writing the file to disk also resolves the issue for me.


Assertion Error on Pico W (RP2040)

Hi there!
Was testing this library with the new Pico W, but I keep getting an error: "AssertionError: b'HTTP/1.1 400 BAD REQUEST'". It looks like the Micropython library had this issue and was fixed and I was wondering if there was a fix for this CircuitPython library?
Any and all help is appreciated!

@AngryChihuahua04 did you ever find a solution to this? Im currently stuck with the same problem

That means the server gave a 400 bad request response.
You can use this in your code to activate the logging of the server's response but that probably doesn't contain more info:
import adafruit_logging as logging
LOGGER = logging.getLogger("websockets.client")
LOGGER.setLevel(logging.DEBUG)

It would be interesting to know what server it is, if it's publicly accessible that would help me test it. If it's a server you are running, you could try to get some debug information on why it's not happy.
It might not like the Origin header, or there's something else. I noticed that it does not give the proper scheme: it always uses http://<host>:<port> even over https, that might be the cause.
This would be this line:
https://github.com/Neradoc/websockets-for-circuitpython/blob/46f78271a286b17def9292af9be26bbfae44650c/uwebsockets/client.py#L62

@Neradoc Thanks for your fast response!
the server expected a connection on wss and port 8002 but I was trying to connect with ws on 8001.
the problem is that the "server" is actually a Samsung TV and when I try to connect with wss, I get the error that the certificate is invalid. Duh. Its self signed on a local device, I get that but is there any way to "look past" the certificate with your implementation?
Thanks in advance :)

Btw, connecting to the TV with Postman using WSS works. It seems to just ignore the invalid certificate, I get no warning

Can you paste here the full error ? (The whole stacktrace with the lines and files and stuff).
I'll have to look into it, I'm not sure of what is supported in the picow implementation currently.

x509-crt-bundle:Failed to verify certificate
OSError: (-12288, 'MBEDTLS_ERR_X509_FATAL_ERROR')
this is all I have unfortunately.. but as I said, I think that's not really a bug but more a feature.
the certificate on the server is bad. but for my project I need to ignore the bad certificate but I didn't find a way to disable the cert check


ISSUE-1054: UI changes in topology view mode for component metrics an…

…d log level
Fix for #1054.
Adds UI side changes for #1023 and #1014 .
This PR does not include UI support for sampling, it will be provided in separate PR which is a WIP.

I pulled all the pending PRs related to this and built & tested.
Below link is the working branch:
https://github.com/HeartSaVioR/streamline/tree/ISSUE-1054-pulling-also-1001-1033-1014-1023
Please note that I've resolved some conflicts and build failures. I'd like to see other PRs sequentially get reviewed merged sooner.

The UI looks like working properly with Chrome, but not Safari and Firefox.
Please take a look at component box from both Overview tab and Metrics tab.

Overview

Chrome

Firefox & Safari (same issue)

Metrics

Chrome

Firefox & Safari (same issue)

I also tested dynamic log level and it worked well.

The Firefox and Safari issue has been fixed.

@joylyn Sorry, but could you follow up the change of API endpoints?
While reviewing PR #1023, we decided to append /metrics to the postfix of topology catalog API endpoint instead of having new root path /api/v1/viewmode/.

/api/v1/catalog/topologies/:topologyId/metrics
/api/v1/catalog/topologies/:topologyId/sources/metrics
/api/v1/catalog/topologies/:topologyId/sources/:sourceId/metrics
/api/v1/catalog/topologies/:topologyId/processors/metrics
/api/v1/catalog/topologies/:topologyId/processors/:processorId/metrics
/api/v1/catalog/topologies/:topologyId/sinks/metrics
/api/v1/catalog/topologies/:topologyId/sinks/:sinkId/metrics

Thanks in advance!

@HeartSaVioR I have made the change for the new APIs.

I'm updating the cluster with changes on #1023 and #1055 (this). Will let you know once I complete.

FYI: updated the cluster.

I had a short talk with @joylyn regarding the rule to decide unit of latency:

less than 1000 (1 sec): represent the value as millisecond
equal or more than 1000 (1 sec): represent the value as second

@shahsank3t @arunmahadevan @joylyn
Please chime in if you have better rule for representing. Thanks!

@HeartSaVioR @joylyn
I believe that's the best way to show the units.
For latency, yes keep ms till value is under 1000 and then show 1 sec until it reaches to minutes & hours.
For others, show plain numbers until it reaches to thousand and then start with k(for thousand), m(for millions), b(for billions) and t(for trillions) hoping it would be rare case to go beyond that ;)
Above has already been implemented and I think we should keep as is unless we have consensus to change it.

+1
@shahsank3t , @joylyn , thanks for the patch. I have done some basic testing and things seems to work well. @HeartSaVioR  may make some changes to address https://github.com/hortonworks/streamline/pull/1032#pullrequestreview-81374655.
After that we can retest this and merge.

A few minor UI issues.

On clicking the show metrics "All components" to change the component, theres a blank entry at the bottom.

On selecting a component here, the "Sampling percentage" box is automatically displayed in the graph. This is not required.

On selecting a subsection of the chart (say input/output), nothing happens. Is something missing?

@joylyn
From #1032 we've allowed metrics from previous to be "null" if there're not enough metric points to be evaluated. (for example, 10 mins time range when topology uptime 15 mins, which current time bucket is for 10 mins but prev time bucket is only for 5 mins)
If UI encounters null from prev*, don't represent trending (+ or -).

On selecting a subsection of the chart (say input/output), nothing happens. Is something missing?

@arunmahadevan Yes we missed on adding a functionality. Basically, if the user selects a subsection, we will update the start/end time for everything and proper show values. Later if the user wants to go back to normal state, they can use date-time range picker on the UI and select timeframe they want to see.

@HeartSaVioR  @arunmahadevan I have made the requested changes.
Please note that since sampling is not yet enabled, the respective buttons is not seen for now.

The earlier comments have been addressed. A few other observations:

After selecting a component, no easy way to "de-select" the component and view the metrics for all. I figured out a way by selecting "All components" from the bottom, but its not intuitive. Maybe on clicking an already selected component, we should de-select it.

After selecting a sub-section of the chart , how do I go back to full view ?

After selecting a component, no easy way to "de-select" the component and view the metrics for all. I figured out a way by selecting "All components" from the bottom, but it's not intuitive. Maybe on clicking an already selected component, we should de-select it.

Yeah, we will add de-select component by clicking already selected component.

After selecting a sub-section of the chart, how do I go back to full view?

You can click on date picker at the top (besides log search icon) and select the timeframe. That's the same time frame that gets changes when you select a sub-section in the chart.
cc: @arunmahadevan  @joylyn

@joylyn also look at the changes @arunmahadevan  mentioned here: https://github.com/hortonworks/streamline/pull/1037#pullrequestreview-82040858

@arunmahadevan  @shahsank3t

The "selection/de-selection" of component on clicking is already implemented.
I have fixed the issue mentioned in #1037 (review)

merged conflicts and rebased.

After selecting a log level and percentage, if I refresh the screen the setting disappears. Is this already addressed in this PR ?

+1


Improve types in Reflector

Discovered while working on injector-php-7

Thanks!

You're welcome.


⚠ Processors has degraded performance

In a795030, Processors ($STATUS_URL) experienced degraded performance:

HTTP code: 200
Response time: 68 ms

Resolved: Processors performance has improved in f8b399c.


Add hmacsha256 dslExpression

Proposed changes

Checklist

[ ] Pull request is created against the dev branch
[ ] All checks passed (lint, unit/integration/regression tests etc.) with my changes
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I have added necessary documentation (if appropriate)

Add hmacsha256 dslExpression


Collection of common port issues

Trying to get a list together of common port issues which have been seen in the past and have been fixed or are still in need of a fix (maybe the fix is already known and it it just must be applied):
Oberserved Problems?

Changing the debug name of a library, find_package broken: #6014
Manually changing the name of a library / dll leading to the problems described in #6553
Changing the exported symbols: #6043
Wrong library linkage: #5543, #5540
packageConfig.cmake in wrong directory: #4783
package*.cmake different for release and debug and release version is overwritten by debug version: (probably also mentioned in #5543 or add a better reference)
single configuration triplets fail because the portfile trys to install/copy files which do not exist in a single configuration build

What should new ports always test?
(after of course building successfully)

Port\Package should be correctly be found by find_package
Linkage of packages should be correct for debug/release (must also be checked in any generated target/config file)
single configuration triplets (either release or debug not both)

Should I ping people for more visibility?
Please add comments so that I can extend the list (and some of you have probably more experience than me fixing ports).

There's also the case when consuming libraries cannot find a dependency due to static and shared libraries having different names.
"Fixable" by using a vcpkg-cmake-wrapper.cmake file, e.g.: rocksdb.

@vicroms

"Fixable" by using a vcpkg-cmake-wrapper.cmake file, e.g.: rocksdb.

Please don't do that. Target RocksDB::rocksdb is probably intended to be always a static target while RocksDB::rocksdb-shared should always be a shared target. I had the same issue with hdf5 once and somebody thought it would be a good idea to redefine the targets. In x64-windows hdf5:hdf5-static was mapped to hdf5:hdf5-shared while in x64-windows-static it was the other way around. If you have problems with consuming packages not thinking about how to link correctly to rocksdb in all cases patch their behavior instead. If rocksdb is building shared as well as static targets (in e.g. x64-windows) do not deactivate that in vcpkg because of the missing target you generate with that. (Furthermore default behavior is unnecessary changed.)

@Neumann-A  thanks for the explanation. I haven't actually done it myself, it's just something I've noticed while reviewing PRs.
I agree, that we shouldn't change the default behavior of libraries, however producing both shared and static libs will fail during post-build validation. We could probably add a variable that should be explicitly set for libraries that produce both libraries, to bypass this post-build validation.

Target RocksDB::rocksdb is probably intended to be always a static target while RocksDB::rocksdb-shared should always be a shared target.

Within vcpkg, it is our policy to "erase" the differences between static and shared targets. The reasoning for this is as follows:

It is critical to avoid symbol duplication to avoid ODR violations
Therefore, there needs to be a single owner for a given symbol (i.e. either a single static or a single shared library)
At the end of the day, it is the process owner (i.e. the person running vcpkg.exe) who should have total authority over all these choices.

Vcpkg provides a mechanism for that user to specify the single (shared/static) binary they want: the triplet file. For most libraries, this works perfectly.
However, libraries (like rocksdb) which change their name between static and shared cause problems. It means that somehow, higher level libraries that build upon rocksdb need to be "told" which of rocksdb's targets to use. In practice, 99% of the time, this configuration is not properly expressed -- when it is switched upon, it usually is done as "if I'm building as static, then of course my dependency must be static". This is obviously over-restrictive (take the extremely common case of building static libraries on top of the shared system copy of openssl).
Thus, leaving things as they are by default simply results in many failed builds for absolutely no reason. We therefore as a policy "erase" these distinctions by aliasing identical targets together, such as in the case of hdf5 and rocksdb. We will still recommend to the user that they use the "appropriate" target name for their configuration (so if you install rocksdb:x64-windows-static we recommend you use rocksdb::rocksdb and similar for rocksdb:x64-windows and rocksdb::rocksdb-shared), however we opt to follow the choice made in the triplet file when these do not exactly match.

@vicroms:

I agree, that we shouldn't change the default behavior of libraries, however producing both shared and static libs will fail during post-build validation. We could probably add a variable that should be explicitly set for libraries that produce both libraries, to bypass this post-build validation.

I think this problem only occurs on -static triplets. In this case the port could try to set BUILD_SHARED_LIBS to OFF and observe if the package/port is doing the right thing (not installing DLLs).
@ras0219-msft:

Within vcpkg, it is our policy to "erase" the differences between static and shared targets.

That breaks downstream expectations. There might be a good reason why a library is actually linked statically with a shared CRT. One of those reasons is DLL-Hell which I don't know if it is still in issue but I saw strange behavior using MATLAB (HDF 1.8.12) and my program at the same time which both use hdf.dll if I link hdf dynamically so I assume yes. (Side Note: My program actual can use both MATLAB APIs and the newer HDF5 APIs (1.10.5), that is why I actually link hdf5-static).
Erasing the difference between static and shared targets is the wrong approach if the difference actual matters. It would be better to delete the other target completely to be consistent (But this of course would require a lot of extra patching).

It is critical to avoid symbol duplication to avoid ODR violations
Therefore, there needs to be a single owner for a given symbol (i.e. either a single static or a single shared library)
At the end of the day, it is the process owner (i.e. the person running vcpkg.exe) who should have total authority over all these choices.

Yes but it isn't solved by erasing the difference within vcpkg (see #5540 -> #https://github.com/Microsoft/vcpkg/issues/5485#issuecomment-468671387). The main culprit here is the lazy linkage (VcpkgAutolink) within the visual studio integration which also requires the manual-link folder (which is prone to error as seen by the last mentioned issue). CMake does not require that because it explicitly links the correct library so there are no symbol clashes.
Furthermore, as far as I know, if a shared library is build properly on windows __declspec(dllexport) is required which adds extra decorations to the exported symbols (correct me if I am wrong here). So the symbols are already different.
If you are redirecting targets you are in reality taking away authority from downstream targets/packages/projects which I think is bad.
In the end in might just come down to change the implementation of how vcpkg achieves the visual studio integration. At the same time you probably also want to fix the /include path and overwriting of headers file be moving everything into /include/packagename. Again this choice was probably made to have a lazy VS integration despite the obvious problems.

Vcpkg provides a mechanism for that user to specify the single (shared/static) binary they want: the triplet file. For most libraries, this works perfectly.

That is already untrue. Because in x64-windows some libraries are build as static libraries and can only be build as that. I assume that mantra lead to, what I call, the great vcpkg dll fiasko #6043. Be a bit more strict with your own guidelines which can be reinterpreted as:

Do not change default package behavior if it is reasonable because you will break downstream targets or expectations

and see x64-windows as all libraries (static/shared) with shared CRT linkage instead.

@NancyLi1013 Don't unpin this.

single configuration triplets fail because the portfile trys to install/copy files which do not exist in a single configuration build

And

single configuration triplets (either release or debug not both)

Wasn't there a single issue thread at one point (not so long ago) to bundle all of those issues together? I was looking for it to check if python3 was on it:
https://github.com/microsoft/vcpkg/blob/master/ports/python3/portfile.cmake
because it has the issue where the port will try but fail to deal with the debug files if there is a triplet stopping debug from being built.
I fixed it in my copy of the tree and was going to push up a patch via merge. But I wanted to check that someone else hadn't flagged it in that thread. Unfortunately, the issue search function is woeful and didn't turn up anything when I tried. The search function worked much better before all the weird changes were made to github.
Does anyone know where the thread is please? (Hopefully maybe someone bookmarked it?)
Maybe it could be linked here by adding it to the thread's parent?

Within vcpkg, it is our policy to "erase" the differences between static and shared targets. The reasoning for this is as follows:

It is critical to avoid symbol duplication to avoid ODR violations
Therefore, there needs to be a single owner for a given symbol (i.e. either a single static or a single shared library)
At the end of the day, it is the process owner (i.e. the person running vcpkg.exe) who should have total authority over all these > choices.

I think this approach will lead to a lot of problems.
I understand that currently there's different triplets: "x64-windows" and "x64-windows-static".
The first issue is: won't there be a lot of duplicate files in these two locations?
For example, the include files and data files, etc. can be the same regardless of static or shared configuration.
Second: some libraries might support static build, some might support shared, and some: both.
So if someone wants to use libabc which is shared-only, and libxyz which is static only, then they need to
reference two different prefix targets: installed/x64-windows and installed/x64-windows-static, and this will be configuration mess.
I understand that one of the motivations is to support vcpkg integrate install.
I suggest to follow a similar approach as Linux (e.g. apt, pacman, etc).
Have one install prefix (e.g. x64-windows).  Put static libs, shared libs, debug libs, etc all in x64-windows/lib.
Add an extra file for vcpkg integration support (similar to how there's an extra file for pkg-config support) under lib/vcpkg/
for example, lib/vcpkg/avcodec.info that has extra information used by vcpkg integrate install, such as the default library for auto-linking via vcpkg integrate install, etc.

The first issue is: won't there be a lot of duplicate files in these two locations?

doesn't matter. Also the VS integration won't work otherwise due to requiring #define SOMEAPI __declspec(dllimport) in the main header of some libraries quite often.

Second: some libraries might support static build, some might support shared, and some: both.
So if someone wants to use libabc which is shared-only, and libxyz which is static only, then they need to
reference two different prefix targets: installed/x64-windows and installed/x64-windows-static, and this will be a messy configuration .

No the solution here is a custom triplet with per port customization. Do not mix different vcpkg installed folders!

Have one install prefix (e.g. x64-windows). Put static libs, shared libs, debug libs, etc all in x64-windows/lib.

won't work since you have not fixed the filename collision of import lib and static libs. You also have problems using find_library and finding the correct one in cmake.. Also you can only ever link one library so it does not make sense to have both!

(similar to how there's an extra file for pkg-config support)

pkg-config on linux is kind of broken considering you have to pass --static without actually knowing if you have to pass it. vcpkg fixes this by never requiring to  --static since the pkgconfig are fixed to reflect the actual select library type.

Oberserved Problems?

Typo.

* single configuration triplets fail because the portfile trys to install/copy files which do not exist in a single configuration build

Now there is only "release" as single-config.

What should new ports always test? (after of course building successfully)

Linkage of packages should be correct for debug/release (must also be checked in any generated target/config file)

This is systematically broken by vcpkg for INTERFACE_LINK_LIBRARIES in the main config file, because debug variant is silently dropped (#22546).

single configuration triplets (either release or debug not both)

Now there is only "release" as single-config.
Additional problems:

The find wrapper does not properly set the policies it needs.
The find wrapper sets <Pkg>_LIBRARY with select_library_configurations to a list, but either the find module (e.g. FindFontconfig.cmake) or a consumer expect a single file path.
pkg-config files missing for some triplets (usually windows).
pkg-config data invalid (e.g. lib names do not match).

We also should run post check in non-Windows.


Sigmoid for Tensors are slow

Mainly due to the fact that Tensor.Apply(sigmoidFn) is slow. There should be a way to optimize this for entire arrays

The plan is to write SIMD versions of vecExp for both Float32 and Float64 tensors. Estimated start time, sometime next week

What is the status on this?

Got sidetracked with the tensor package refactor. However, now application is fairly trivial. Will push the changes thru in a couple of days.

More delays on this - CUDA support sidetracked me once again

Solution to this for now is to us CUDA. Will close this for now.


Sync eng/common directory with azure-sdk-tools for PR 6817

Sync eng/common directory with azure-sdk-tools for PR https://github.com/Azure/azure-sdk-tools/pull/6817  See eng/common workflow

@scbedd I think the failures here are due to the fix in https://github.com/Azure/azure-sdk-tools/pull/6671 to properly pipe the error output into stderr. It looks like there are a bunch of tests that are now failing due to the checking that the Test Framework does to ensure stderr is empty at the end of each test. It seems that most of the errors involve a recording file not existing, but it seems to be a race condition as it only happens on some platforms.

Since the admin logging PR fixes a regression, I wonder if we can get a proxy release with just that commit, and work on investigating the failures caused by the exception logging fix separately?

I don't really understand what you're saying here?

I wonder if we could get a proxy release with just that commit

Do you mean, with just the commit fixing the regression, but without my fix on stderr?

Do you mean, with just the commit fixing the regression, but without my fix outputting to stderr?

Yes, that is correct.


revoke doesn't remove generated client file

If it is installed at /home/user/... the revoke function doesn't remove the generated client file.
${HOME} refers to /root/...
https://github.com/angristan/wireguard-install/blob/4dfbc55cd91861c357c5ff07146c8639fcb89c61/wireguard-install.sh#L357

Fixed with https://github.com/angristan/wireguard-install/commit/7565f26b5d2e4542f777001859970378704ce288


python3Packages.sqlalchemy-views: init at 0.3.2

Description of changes
Initialize the sqlalchemy-views package at version 0.3.2.
Things done

Built on platform(s)

[x] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

[ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
[x] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
23.05 Release Notes (or backporting 22.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[x] Fits CONTRIBUTING.md.

@mweinelt Can this be merged in?

@ofborg build python3.pkgs.sqlalchemy-views
@ofborg build python311.pkgs.sqlalchemy-views


Workaround for memory issue of *>

This attempts mitigating the possible effects of the issue https://github.com/typelevel/cats-effect/issues/401 that's still open.

Codecov Report

Merging #72 into master will increase coverage by 0.04%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   22.94%   22.99%   +0.04%
==========================================
  Files          33       33
  Lines         950      948       -2
  Branches       16       17       +1
==========================================
  Hits          218      218
+ Misses        732      730       -2

Impacted Files
Coverage Δ

...src/main/scala/laserdisc/fs2/RedisConnection.scala
0% <0%> (ø)
:arrow_up:

fs2/src/main/scala/laserdisc/fs2/RedisClient.scala
0% <0%> (ø)
:arrow_up:

cli/src/main/scala/laserdisc/cli/CLI.scala
0% <0%> (ø)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 282c4bd...f9783ac. Read the comment docs.


A specification for the standard library functions (StringLib, so far)

This partial test suite already serves to document the behavior of the functions defined in StringLib (excepting Search), and it verifies that both the typers/simplifiers and the compilation to MongoDB's pipeline are correct.
The pipeline runner identifies which functions are not implemented for the pipeline and those tests show up as "skipped" tests in the log.
It's expected that we'll follow this up with:

tests (examples) for the rest of the standard library (well, the MapFuncs at least).
test runners for MongoDB JS and Selector compilation
test runners for additional backends, or for sub-components of them

Are you telling me our StringLib implementations were already correct? Unpossible!

Yes and no. There are some big caveats.
I restricted the domain of strings for MongoDB to just alphaChar because $toLower and $toUpper are defined only for ASCII (and I'm fixing it to be ASCII now). It's a bummer that there's no easy way to override the domain for a particular test.
Half of these String-related functions aren't implemented in aggregation ops anyway.
I declawed the Substring test because the simplifier blows up if it gets any invalid index. That one's an easy fix, so I'll do that now, too.
Integer and Decimal handle only Int and Double in the simplifier/typer, so I'll fix that, too.
Other than that, yeah, pretty much correct.

@mossprescott LGTM, couple of minor comments I'll wait to hear from you about.

Restarted build job as it failed due to a reported race-cond in services spec regarding port usage.

Current coverage is 69.83% (diff: 90.47%)

Merging #1396 into master will increase coverage by 0.22%

@@             master      #1396   diff @@
==========================================
  Files           253        254     +1
  Lines         12928      12958    +30
  Methods       12647      12663    +16
  Messages          0          0
  Branches        143        142     -1
==========================================
+ Hits           8999       9049    +50
+ Misses         3929       3909    -20
  Partials          0          0

Powered by Codecov. Last update 8f9fd29...3bca835

@mossprescott LGTM, CHANGELOG and I'll merge.


Feature request: WYSIWYG (markdown) editor for comany and presentation details

We need rich text for description fields like company details.
Suggestion:

Find a (JavaScript) rich text editor that supports markdown (like the one used in GitHub?)
Create a Blade and/or Livewire component that can 'replace' current <textarea>s.

Use this component just like a textarea:
<x-rich-textarea id="description" wire:model="form.description"
                              class="..."
                              name="description" required
                    ></x-rich-textarea>

It seems like we'd have to make some sort of trade-off if we decide to use one. For now the viable options seem to be:

SimpleMDN
Milkdown
WYSIMARK

All of them are JS editor which makes styling a pain. SimpleMDN is rather outdated (there haven't been any activity over 8 years there); Milkdown is actively in development (but the plugins are not matching our needs exactly e.g. the menu) and WYSIMARK again is hard to customize.

PR #532 introduces a markdown editor that is a component so can be used in 2025.

Dup #142


[ESP32] Configure the event logging buffers to reduce dram overflow i…

…n all-clusters-app for m5stack
DRAM is overflowing... this is the temporary fix which reduces the event logging buffers and being tracked in #34717

Hotfixing ToT build breakage.


[SETUP-BUG] Pre-commit fails (Apple Silicon Mac)

System information

OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Darwin Kernel Version 21.3.0: Wed Jan 5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_ARM64_T6000
FiftyOne installed from (pip or source): source
FiftyOne version (run fiftyone --version): 0.14.3 (develop 7c389522ab2122887e69d994214ecc4440bc371f)
Python version: Python 3.9.10

Commands to reproduce
After failing to build developer (refer to https://github.com/voxel51/fiftyone/issues/1584), I tried to run install.bash manually
pre-commit install
pip install -e .

Make some code change and try to commit
git add <files>
git commit

git commit                                                                                daniel@Daniels-MacBook-Pro
Error processing line 1 of /Users/daniel/miniforge3/envs/pytorch/lib/python3.9/site-packages/distutils-precedence.pth:

  Traceback (most recent call last):
    File "/Users/daniel/miniforge3/envs/pytorch/lib/python3.9/site.py", line 169, in addpackage
      exec(line)
    File "<string>", line 1, in <module>
  ModuleNotFoundError: No module named '_distutils_hack'

Remainder of file ignored
[INFO] Installing environment for https://github.com/prettier/prettier.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('/Users/daniel/.cache/pre-commit/reposi83g34u/node_env-system/bin/node', '/Users/daniel/.nvm/versions/node/v17.3.1/bin/npm', 'install', '--dev', '--prod', '--ignore-prepublish', '--no-progress', '--no-save')
return code: 1
expected return code: 0
stdout: (none)
stderr:
    npm WARN install Usage of the `--dev` option is deprecated. Use `--include=dev` instead.
    npm ERR! code ERESOLVE
    npm ERR! ERESOLVE unable to resolve dependency tree
    npm ERR!
    npm ERR! While resolving<EMAIL_ADDRESS>    npm ERR! Found<EMAIL_ADDRESS>    npm ERR! node_modules/rollup
    npm ERR!   dev<EMAIL_ADDRESS>from the root project
    npm ERR!
    npm ERR! Could not resolve dependency:
    npm ERR! peer<EMAIL_ADDRESS>from<EMAIL_ADDRESS>    npm ERR! node_modules/@rollup/plugin-alias
    npm ERR!   dev<EMAIL_ADDRESS>from the root project
    npm ERR!
    npm ERR! Fix the upstream dependency conflict, or retry
    npm ERR! this command with --force, or --legacy-peer-deps
    npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
    npm ERR!
    npm ERR! See /Users/daniel/.npm/eresolve-report.txt for a full report.

    npm ERR!      /Users/daniel/.npm/_logs/2022-02-04T00_24_37_309Z-debug-0.log

Check the log at /Users/daniel/.cache/pre-commit/pre-commit.log

Describe the problem
Seems removing prettier section in .pre-commit-config.yaml would let it pass.
Other info / logs
Logs pasted above.

Think this is OBE


Manifests and Files tables for Iceberg Connector

Spec: https://iceberg.apache.org/spark/
Umbrella issue: #1324

Thanks!


Unable to upload profile picture

Bug report
Unable to upload a profile picture
Preconditions
1. Latest

Steps to reproduce
1. Go to user profile
2. Click on edit profile

Expected result
User can upload their photo

Hi @AkashWeybee
Right now, this functionality is not in bagisto. This is a static text written there.

Will you accept pull request for this feature?
Need to add a column in the customer table.

Hi @AkashWeybee
You can create PR for the same but make sure to create new migration for the new column.

Is this still open? I can work on this if it is.

Sure! It's open. Go ahead.

@devansh-webkul
I am reopening this issue because this feature is not implemented in the default theme.Please find the screenshot Here

@devansh-webkul
As we discuss this feature is only for the velocity theme, so I am closing this issue


fix: fix test file name

😞

Codecov Report

Merging #789 (4027f74) into main (f86f76d) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #789      +/-   ##
==========================================
- Coverage   52.51%   52.47%   -0.05%
==========================================
  Files         116      116
  Lines        6628     6628
==========================================
- Hits         3481     3478       -3
- Misses       2790     2792       +2
- Partials      357      358       +1

Impacted Files
Coverage Δ

header/core/listener.go
52.83% <0.00%> (-5.67%)
:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f86f76d...4027f74. Read the comment docs.


🐛Bug: Popover remains open during development

Describe the bug
While styling a popover, I would open it to see what it looks like, then head back to my editor and update it.
There is a bug that when the popover is open, and an change is made to the page, the popover will 'glitch', move to the top left of the screen and remain open.
An example of this can be seen here -> https://stackblitz.com/edit/node-fdgwxx?file=src%2Froutes%2F%2Bpage.svelte,.prettierrc)
To Reproduce
Steps to reproduce the behavior:

Open the stackblitz example
open the popover
make a change in the editor (while the popover is still open) and save

Expected behavior
The popover is destroyed due to a reload
Screenshots
n/a
Desktop (please complete the following information):

OS: Mac OS
Browers: Firefox, Safari, Chrome

Smartphone (please complete the following information):
n/a
Additional context

The same things happens with dialog

@geerew The issue is not happening with me on the latest version of Melt UI. Can you confirm? Thanks 😄


feat(kong-ngx-build) apply NGINX core patches for HTTP/2 and mp4 modules CVEs

These patches allow our build script to produce OpenResty <IP_ADDRESS>, <IP_ADDRESS>, and <IP_ADDRESS> without the recent vulnerabilities found in HTTP/2 and mp4 modules.

Looks like this PR caused some build fail to compile:
https://travis-ci.com/Kong/openresty-build-tools/jobs/224977025

@dndx I did not include the <IP_ADDRESS> ports of the patches since only old versions of Kong are built on top of the 1.11.2 NGINX core. The patches are available online though.

@thibaultcha If that's the case should we remove 1.11.x from the matrix? Otherwise the test will always looks red.

@dndx I forgot that we added 1.11.x to the CI matrix. Adding the 1.11.2 patches to this repository then.


Python RxPy

Python with RxPy (and AIOHTTP):

Python's functional reactive streaming/ReactiveX/RX implementation
The weirdest of the functional reactive streaming options (compared to Akka Streams, Combine and Flow, which are largely similar)

For some reason, they decided to move from method-chaining to their own pipe-style chaining. This style doesn't strike me as being more Pythonic, is more verbose, and definitely limits what the type-checker can do 🤷

Nice! Yeah, the pipe is odd and I wonder why they couldn't use async await instead.


Removed DB submodule

Removed the DB submodule and copied files worked until now into normal directory.

the security hotspots are in test files


[zeromqcpp] from 4.3.1, use recv taking a reference to message_t and recv_flags

Here is a problem about zeromqcpp.
In the laste version, we can see:
#ifdef ZMQ_CPP11
    ZMQ_DEPRECATED("from 4.3.1, use recv taking a reference to message_t and recv_flags")
#endif
    bool recv(message_t *msg_, int flags_
#ifndef ZMQ_CPP11
              = 0
#endif
    )
    {
        int nbytes = zmq_msg_recv(msg_->handle(), _handle, flags_);
        if (nbytes >= 0)
            return true;
        if (zmq_errno() == EAGAIN)
            return false;
        throw error_t();
    }

flags_ cannot be automaticlly set to be 0 when using "-std=c++11", and we need to pass 0 when calling recv.
However, in this way, the system cannot work right. It shows error like:
./build/kvs/client/cpp/terminate called after throwing an instance of 'zmq::error_t'
  what():  Operation cannot be accomplished in current state

I am now trying another solutions is to use an older version of zeromqcpp and will update here.

An older version of zeromqcpp will remove the compile issue, but the system still works incorrect.
My step likes following:
> ./scripts/start-kvs-local.sh n n
> ./build/kvs/client/cpp/flkvs-cli ./conf/kvs-config.yml
kvs> GET
terminate called after throwing an instance of 'std::logic_error'
  what():  basic_string::_M_construct null not valid
Aborted
terminate called after throwing an instance of 'zmq::error_t'
  what():  Operation cannot be accomplished in current state

By the way, we cannot find "./build/kvs/src/cli/flkvs-cli", I guess it may change to another location, i.e., "./build/kvs/client/cpp/flkvs-cli".

Thanks for pointing this out, @authwork. It seems like this is an accidental misconfiguration in the latest version of ZMQ, as you can see here. I explicitly set the ZMQ version to 4.2.3 on my local machine, and that seems to have fixed the issue. I'll make a PR, and we'll see if it fixes it in the Travis context as well.


Clear statement after exposure submission that others were warned

Current Implementation
Currently, the app only makes vague statements that others were warned after the "Warn others" process was finished successfully. Actually, the only thing I found was this message: "Thanks to your support, other people can now be warned and respond appropriately.".
Suggested Enhancement
Make a clear statement whether others were already warned or not, like.
"The process to warn others was finished successfully and other people are now warned.".
Expected Benefits
I wrote with quite a few users on Twitter via DM who were unsure if the app did warn others already, or if they still have to do anything. This is especially a problem if the app warned automatically in the background.

@Ein-Tim Thanks for the suggestion.

As the CWA project went into ramp-down mode, I don't expect this feature to be implemented. I'm therefore closing this issue.


Support multiple output channels for log message notifications

Proposing a feature to allow log message notifications to report their output to a specific channel.  These could be used to differentiate the output between multiple long and short lived tasks that may be run by the same language server at any given time.
Currently, since all log message notifications end up mixed in the same output channel, it can be difficult to use for cases such as implementing a run of a build that produces its own linear output that should be collected and viewed separately from any of the server's other output.
Another option could be something more like a show document request that displays text in a more read only manner, for example in a display more similar to the view in which VS code outputs test run progress.

See also:

https://github.com/microsoft/language-server-protocol/issues/2014


Responsive corrections

Summary
Responsive fix to DownloadPageComponent
Fix responsive to SearchbarComponent
Limit the number of restaurants displayed in PopularRestaurants, rendered in HomePageComponent
Some bugs presented in RestaurantFilterComponent are corrected
A new component is created to render each of the RestaurantFilterComponent checkboxes
Fix RestaurantFilterComponent styles improving responsiveness
The styles of the restaurant list are modified
The function of filtering by type of cuisine is restored
Header styles are modified

Look good to me, although some of the changes I think were addressed on the restaurant filter patch branch I did earlier

All comments were considered applied


fix: file extension stripping in writeTypes function

🔗 Linked issue

❓ Type of change
  const connectPostgresJS: typeof import('../../../../node_modules/.pnpm/@pergel+nuxt@0.7.19_nuxt@3.9.3_rollup@4.9.6_vite@5.0.12_vue@3.4.15/node_modules/@pergel/nuxt/dist/runtime/modules/drizzle/drivers/postgres/export.mjs')['connectPostgresJS']

fixed pr
  const connectPostgresJS: typeof import('../../../../node_modules/.pnpm/@pergel+nuxt@0.7.19_nuxt@3.9.3_rollup@4.9.6_vite@5.0.12_vue@3.4.15/node_modules/@pergel/nuxt/dist/runtime/modules/drizzle/drivers/postgres/export')['connectPostgresJS']

[ ] 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
[x] 🐞 Bug fix (a non-breaking change that fixes an issue)
[ ] 👌 Enhancement (improving an existing functionality like performance)
[ ] ✨ New feature (a non-breaking change that adds functionality)
[ ] 🧹 Chore (updates to the build process or auxiliary tools and libraries)
[ ] ⚠ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

📝 Checklist

[x] I have linked an issue or discussion.
[x] I have updated the documentation accordingly.

@danielroe fixed.

Thank you ❤


dao: update function: generated SQL was incorrect

generated SQL was incorrect, because of handling of virtual fields

LGTM


Add "ASK" responses to SQRL requests

The SQRL protocol set out that when a server has set the TIF to be "IdMatched" (0x1) that it may respond with an "ASK" in the body of the response. This can allow a server to ask critical questions or validate intent within the SQRL client. This would be useful if the server has a need to authorise requests independently to a login request.
Proposal
Add to the options for the middleware the ability to setup a function that will be called with request information so that the server can determine if they wish to send an ASK in the response. These function would need to return a standard object with the following details.
public class AskMessage
{
    public string Message { get; set; }
    public AskMessageButton Button1 { get; set; } //This can be null
    public AskMessageButton Button2 { get; set; } //This can be null

     public class AskMessageButton
     {
          public string Text { get; set; }
          public string Url { get; set; } //This is optional
     }

}

When the client responds with the users choice (the BTN parameter) another method will be called with the original requests details and the button (button 1 or button 2) that was pressed this can allow the server to do other things with that.
When a URL is given the user should be shown that page on the device that the SQRL client is running. This URL is relative to the host site.

It might be needed that the middleware needs to listen to other URL for this to work other than the standard callback URL.
I think we should allow developers to specify more than one path for instance a site might have
Callback path: "/login-sqrl" (default callback path)
But also want to run logins on these URLs as well
"/checkout/uservalidate"
"/profile/update"
These are jsut examples but I can see that if a developer wished to let the middle ware treat this as a login system page that it would be helpful not only for ASK but for others cases.
They can then use this known list when a response to an ASK request comes in to determine if they need to run an action.