ng-rc5 branch: NgModule AppModule uses AppComponent via "entryComponents" but it was neither declared nor imported! This warning will become an error after final

When I download and run ng-rc5 branch, the console show this massage:
CONSOLE WARN file:///app/tns_modules/@angular/core/src/facade/lang.js:314:17: NgModule AppModule uses AppComponent via "entryComponents" but it was neither declared nor imported! This warning will become an error after final.

Hey @Arthurisme,
Thanks for reporting this. We addressed the warning when upgrading to Angular RC 6, and that code is now in the master branch. Please let us know if you run into any other issues.


Null aware operator used on non null able type warning when running

After updating to flutter 3.0 I'm getting a warning about a null aware operator used on a non null type
Here is the warning
: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.
../…/lib/mvvm_builder.widget.dart:47
- 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../fvm/versions/stable/packages/flutter/lib/src/widgets/binding.dart').
package:flutter/…/widgets/binding.dart:1
    WidgetsBinding.instance?.addObserver(this);
                   ^
: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.
../…/lib/mvvm_builder.widget.dart:48
- 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../fvm/versions/stable/packages/flutter/lib/src/widgets/binding.dart').
package:flutter/…/widgets/binding.dart:1
    WidgetsBinding.instance?.addPostFrameCallback((_) => _vm.onMount());

                   ^
: Warning: Operand of null-aware operation '?.' has type 'WidgetsBinding' which excludes null.
../…/lib/mvvm_builder.widget.dart:78
- 'WidgetsBinding' is from 'package:flutter/src/widgets/binding.dart' ('../../../fvm/versions/stable/packages/flutter/lib/src/widgets/binding.dart').
package:flutter/…/widgets/binding.dart:1
    WidgetsBinding.instance?.removeObserver(this);
                   ^

I just write this PR to solve that following that WidgetsBinding.instance is not null able docs

Hi @rurickdev great work my friend
The PR has been approved


See also section contains a duplicate link to ItemTemplateSelector

There's a duplicate link in the See also list for the ItemTemplateSelector doc.

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 86cbc84f-1578-5874-584f-c70bc5a12db2
Version Independent ID: a4782a0f-fe7d-dcec-7ac6-ecd9d0d8359d
Content: DataTemplateSelector Class (Windows.UI.Xaml.Controls) - Windows UWP applications
Content Source: winrt-api-build/xml/Windows.UI.Xaml.Controls/DataTemplateSelector.xml
Product: uwp
Technology: winui
GitHub Login: @jwmsft
Microsoft Alias: jimwalk

Thanks for letting us know about this. A fix has been submitted.


Create proposal to add changelog attribute support to cargo

Sorry, missclick - this was intended to be a merge request on my own repo


DSA: Sorting Algorithm of Count Sort and Radix Sort

I want to write about the Count Sort and Radix Sort in the section of Searching and Sorting Algorithms. Please assign to me.
-> I will discuss the code.
-> Complexity Comparison (Time and Space)
-> Why? and What?
BATCH - 3

I want to contribute towards this issue.please assign it to me
AYUSHI CHAUHAN
BATCH-1
@nikhil00shinde

Mansi Singh and dsa batch-4 pls assign this issue to me

Pls assign this issue to me
Udit katyal
Batch 3 DSA

/assign


Install : The term 'Install' is not recognized as the name of a cmdlet, function, script file, or operable

I am following you "Get Started Document" from
https://playwright.dev/docs/intro#installation
I Installed the Visual Studio extension and the next thing i tried is this, but got the following error
PS C:\Users\nssid> Install Playwright
Install : The term 'Install' is not recognized as the name of a cmdlet, function, script file, or operable
program. Check the spelling of the name, or if a path was included, verify that the path is correct and try
again.
At line:1 char:1
+ Install Playwright
+ ~~~~~~~
    + CategoryInfo          : ObjectNotFound: (Install:String) [], CommandNotFoundException
    + FullyQualifiedErrorId : CommandNotFoundException

The vscode extension is only compatible with the Node.js version of Playwright.
See here for the .NET getting started: https://playwright.dev/dotnet/docs/intro#installation

Sorry for the typo there, I installed visual studio code extension.
I don't want to use .NET, but instead of node.js
The article above which i am trying to follow is for Node.js

It's a command which you need to invoke with the vscode command palette, not the terminal.
See here: https://code.visualstudio.com/docs/getstarted/userinterface#:~:text=Command Palette%23,for the most common operations.

It's a command which you need to invoke with the vscode command palette, not the terminal.
See here: https://code.visualstudio.com/docs/getstarted/userinterface#:~:text=Command Palette%23,for the most common operations.

@mxschmitt, Thanks that helped


Problem at home

Description:
When the user cliks on the banner, its get a 500 error

Resolvemos o problema no pull request #2


data-turbo-permanent does not work on Favicon

Hey,
on my Website, I have a favicon (rel="icon") that gets replaced with an animated version with JavaScript, but after a Page navigation it gets reset to the static version. I have tried using data-turbo-permanent with an id attribute, but it gets replaced nonetheless.
Is there any other way to ignore this element with Turbo?

Hi @sotasan.
The favicon (the icon on the browser's tab) is not being replaced here. It stays static since the first page load.
Could you confirm?

Hey,
yes I am sorry, my fault. I had temporarily removed those lines. I added them now back in. Ah and favicon gifs only work in Firefox.


Can we do segmentation over whole 3d CT dataset without any ground truth?

Hi @aaekay ,
It depends on what you want to segment. SAM is a promptable segmentation method. Thus, you need to feed a prompt to the model to specify the organ that you want to segment.
If you are looking for a pre-trained model that can segment the main organ in CT scans. Totalsegmentator would be a better choice.
https://github.com/wasserth/TotalSegmentator


⚠ Samsung has degraded performance

In 3b77251, Samsung (https://samsung.com) experienced degraded performance:

HTTP code: 200
Response time: 2122 ms

Resolved: Samsung performance has improved in 15cd7b7.


As seen in the demo (same animation is necessary to have the character as well（Goblin_m', 'Goblin_m）or（BigVegas', 'BigVegas）?

As seen in the demo (same animation bvh file exists to both input and destination character) with intra, is it necessary to have the same animation's bvh file for the destination character as well（Goblin_m', 'Goblin_m）or（BigVegas', 'BigVegas）?
how to slove the problem？ @ @kfiraberman

See #59.


Weird behavior of completion

This is a wonderful project, as well as kubectl-aliases, and I love both of them very much !
I've two box, macOS Mojave and Ubuntu Xenial. My oh-my-zsh has something like this:
# .zshrc
...
[ -f ~/.kubectl_aliases ] && source ~/.kubectl_aliases
function kubectl() { >&2 echo "+ kubectl $@"; command kubectl $@; }
alias kns='kubens'
alias kctx='kubectx'
source <(kubectl completion zsh)

On MacOS:

both kns <TAB> and kctx <TAB> work like a charm
kex <TAB> shows nothing

On Ubuntu:

kex <TAB> works perfectly
kns <TAB> & kctx <TAB> do not work, even if I tried #35; _k<TAB> only shows _kubectl neither _kubens nor _kubectx

Any ideas ?

I don’t think bash does completion on aliases out-of-the-box.
Assuming you use zsh on both machines, I think you might not have installed the zsh completion scripts on Ubuntu correctly. I personally don’t have a Linux box so I’m not sure. Maybe some package is missing, maybe the zsh comp files aren’t in the right location.
I’m inclined to say this is not a kubectx issue (although I wish we had a deb pkg so people didn’t have to deal with this).

You are right !
Exec autoload -U compinit && compinit then _kubectx works on Ubuntu perfectly !


Parameterize backend URL

There are currently 9 places where we have the full url for the backend API - including one in .env where it belongs. https://github.com/civictechindex/CTI-website-frontend/search?q=herokuapp.com
Please convert the pages to use the REACT_APP_API_URL envvar so one can change the domain in one place when checking to see if backend changes break the frontend.
NOTE: this issue is a blocker for issue #484

Please update the following
•	Progress: How much progress have you made on the issue?
•	Blocks: Are you facing any blockers?
•	Availability: How much time do you have to work on it this week?
•	ETA: When do you think it will be completed?

I am working on issue #464. After finishing that will work on this


Feature request: Automated task selection

It'd be pretty neat if, when given a slayer task, the plugin parses the relevant ChatMessage and automatically selects the correct slayer task.
Would it be worthwhile to submit a PR for this?

That is a great idea and would be a nice addition.
I think it'd be best to have the featuee enabled by default, but allow the player to enable / disable it as well.

Great! I put a bit of work into it today. Had to brush up on regex patterns to get it to recognize a task assignment. Got it mostly working (it uses SlayerPluginPanel.openTask and closeTask) but have to solve some problems with threads before it's ready to go. Calling it a day before I get any angrier at it =)

@Fiffers This is what I did in my plugin to check what the current task is. You get the name back then I used a dictionary to translate it into the id.
You'd probably want to add bosses too. For example, Vorkath when you get blue dragons etc.
https://github.com/ffalor/slayer-wiki/blob/429c464e8ac71627bcd91a3293b4c05a8f009450/src/main/java/com/slayerwiki/SlayerwikiPlugin.java#L229-L245


geo-query is not compatible with composer/installers newer than ~1.0.0

I install WordPress with Composer (thank's to the awesome Bedrock tool). Bedrock already uses composer/installers v ^1.4. geo-query depends on composer/installers too, it requires version ~1.0.0 that causes a conflict.
When trying to add  geo-query to my project Composer complains that:
Problem 1
    - Installation request for birgir/geo-query ^0.0.6 -> satisfiable by birgir/geo-query[0.0.6].
    - birgir/geo-query 0.0.6 requires composer/installers ~1.0.0 -> satisfiable by composer/installers[1.0.x-dev, v1.0.0, v1.0.1, v1.0.10, v1.0.11, v1.0.12, v1.0.13, v1.0.14, v1.0.15, v1.0.16, v1.0.17, v1.0.18, v1.0.19, v1.0.2, v1.0.20, v1.0.21, v1.0.22, v1.0.23, v1.0.24, v1.0.25, v1.0.3, v1.0.4, v1.0.5, v1.0.6, v1.0.7, v1.0.8, v1.0.9] but these conflict with your requirements or minimum-stability.

Does it really mean that geo-query requires composer/installers that's not newer than ~1.0.0? Or is it just an oversight in the composer.json file? I suppose that it's safe to upgrade that dependency.

@wujekbogdan thank you for bringing that up.
We should definitely update the composer.json here.
I'm not using anything special from the autoloaders that need to restriction to older versions.
I'm thinking ^1.0.0 instead of ~1.0.0, so the range would be >= 1.0.0 and < 2.0.0, to stay within possible breaking changes of version 2.
Thoughts?

Changelog
0.0.7 (2018-06-27)

Fixed #10. Use ^1.0.0 for composer installer. Props @wujekbogdan

Thanks


ChatIPCC

www.chatclimate.ai

ChatClimate uses "tuning" towards first pulling answers from IPCC report before open knowledge of OpenAi. We can do the same and add hyper locality from hyper-local data sets, national and local strategy docs.


co.json: Allow interface types from generics

The co.json type factory accepts a generic to let the Jazz users define the expected shape.
We want to make it possible to pass as argument only types that can be serialized, which means that anything that includes functions shouldn't be accepted as valid type.
The current implementation doesn't accept valid interface types, leading to some confusion when trying to use co.json correctly.
We want to fix the generic type in order to work in the following way:
type ValidJSON = {
    prop: string;
    prop2: number;
}

// This is valid ✅
co.json<ValidJSON>();

type NotValidJSON = {
    date: Date;
}

// This isn't valid ✅
co.json<NotValidJSON>();

type NotValidJSON2 = {
    fn: () => void;
}

// This isn't valid ✅
co.json<NotValidJSON2>();

interface ValidInterface {
    prop: string;
    prop2: number;
}

// This is flagged as error but should be valid ❌
co.json<ValidInterface>();

I came across this discussion in SvelteJS some time ago: https://github.com/sveltejs/kit/issues/1997. I can share some background info.
There are many behaviour differences between interfaces and type literals in Typescript and unfortunately this is one of them. This issue is rooted in the design choice of Typescript where interfaces do not carry an implicit index signature while type literals do. See https://github.com/microsoft/TypeScript/issues/15300. ^
In other words, co.json accepts ValidJSON because ValidJSON’s implicit index signature is the same as the index signature in JsonObject. There are workarounds that involve creating utilities to sort of convert interfaces into types. But there is still no straightforward way to apply these utilities automagically.
From a DX perspective, is providing a utility along with clear instructions and examples for devs sufficient?
^ Internally, it used to be done by a check in getImplicitIndexTypeOfType (release-2.0/src/compiler/checker.ts#L4532) when implicit index signatures were first introduced in 2016, but it has become a part of type inference since around version 4.4, see isObjectTypeWithInferableIndex (v5.6.2/src/compiler/checker.ts#L25083).

Thanks @milkcask!

From a DX perspective, is providing a utility along with clear instructions and examples for devs sufficient?

We can give it a try, do you have any examples that we could follow?
For now we are suggesting this as workaround which actually turns off our checks:
export class MyCoMap extends CoMap {
    data = co.json<unknown>() as co<MyInterface>
}

Anything better than this is highly appreciated.

Found the answer in the linked issue, but our use case is a little more tricky because it is a property definition.
type Typify<T> = { [K in keyof T]: Typify<T[K]> };

interface ValidInterface {
    prop: string;
    prop2: number;
}

export class MyCoMap extends CoMap {
    // This works, yay!
    data = co.json<Typify<ValidInterface>>();
}

const data: ValidInterface = {
    prop: "test",
    prop2: 123,
};

// This doesn't validate and would need casting
MyCoMap.create({ data }, { owner: me });

@gdorsi Your snippet lgtm and should work. What is tsc complaining about on your end?

I usually use this to avoid unnecessary recursion:
type InferIndexSignature<T> = { [K in keyof T & keyof T]: T[K] };

@milkcask Confirm that it works!
Not sure why it didn't this morning 🤔
Probably it was the VSCode cache playing pranks to me 🤷
Thanks for the suggestion and for double-checking it!

@aeplay
I think that we can proceed as @milkcask suggested and provide an utility type + some documentation to let our users handle this use case.
WDYT?

I wonder why we can't do a conditional Generic gate like Typify<T> extends JsonObject ? Typify<T> : never
Am I missing something?

@aeplay
Do you mean something like this?

I had the same idea, but it means that the error wouldn't be flagged when passing the type into the generic but when calling the create function making the type error harder to debug.

I think the homomorphic version of Typify / InferIndexSignature can be adjusted to extends (or intersect with) JsonValue.
type TypifiedJsonValue<T> = { [K in keyof T & string]: T[K] extends JsonValue | undefined ? T[K] : never };

json<T extends JsonValue | TypifiedJsonValue<T>>(): co<T> { ... }

It’s interesting that this works.
@aeplay Yes, that should work too, but a drawback is that when an error occurs, the error elaboration stops at the bare never. This leads to cryptic diagnostic messages like Type 'T' does not satisfy the constraint 'never' rather than Type '() => void' is not assignable to type ....

@milkcask
Didn't know that was possible to use the generic as argument for the extend 🤯
This opens up a possible solution for us!
I've made a quick draft, needs more tests but seems to work: https://github.com/gardencmp/jazz/pull/492/files

@gdorsi That's one of the many undocumented uses of typescript and was added because Immutable.js needed it.
See https://github.com/Microsoft/TypeScript/issues/25379.
As seen in that issue, it can be combined with recursion but there isn't a straightforward way to do it with plain interfaces (unlike IImmutableMap, where you still need to manually create an instance). So I guess your solution is sufficient for most use cases.
Quite an interesting problem.


eel.show should accept more parameters like size

Hello! Today i was trying to create multiple windows using eel with diferent sizes, and i run into a little nigthmare trying to figure out how to do it!
So i would like to ask for a new feature, it would be nice if when you create a new window using:
eel.show(start_urls)
that you could also give it some parameters, something like
show(*start_urls,**kwargs))
to define the params for a new window, defining the size or close callback of it for example

Hi @ithan
I am interested in this enhancement myself and had to try around a little longer because I am not so experienced yet. But I found a solution for those who want to give eel.show() more parameters, like size=....
The __init__.py file of eel must be modified for this. You can find this file in the module installation folder of eel. e.g. C:\Users\<Username>\AppData\Roaming\Python\Python37\site-packages\eel
Change line 140:
show(*start_urls)

To this:
show(*start_urls, **kwargs)

And line 167:
def show(*start_urls):
    brw.open(start_urls, _start_args)

To this:
def show(*start_urls, **kwargs):
    _start_args.update(kwargs)
    brw.open(start_urls, _start_args)

Now you can call the eel.show() to open new windows with extra paramters:
eel.show('hello.html', size=(400, 200), position=(40, 40))

Sounds like a reasonable enhancement - I'll investigate feasibility this weekend, but I don't think it should be too difficult.

Sorry for just getting back to this. Is it just size and position that people would hope to pass on a case-by-case basis? I don't see many of the other params applying as they're mostly global config.

I also believe that passing the size and position parameter would be quite sufficient for everyday use.


Fix String cast mapping

Fixes #1106

The ToChar operator doesn't seem to work as documented.

Now .append("test") chains don't work at all anymore.
Maybe add a trim?

I added now a test for append usage.

And added now the necessary trim as well. At least HSQLDB and Derby required it.

That is indeed what I noticed.

So, are you content with the outcome? Shall I merge then?

Yes, this is fine with me.


AutoUpdate not detecting all winget packages inside included_apps.txt

It looks like not all winget packages inside included_apps.txt get detected and therefore get skipped when updating.
e.g. git:
# file: updates.log
##################################################
#     CHECK FOR APP UPDATES - 13.09.2022
##################################################
19:16:44 - Notification Level: Full. Notification Language: Deutsch
19:16:44 - Checking internet connection...
19:16:44 - Connected !
19:16:49 - Winget Version: v1.4.0-preview
19:16:49 - WAU current version: 1.14.2
19:16:49 - WAU AutoUpdate is Disabled.
19:16:49 - WAU uses White List config
19:16:49 - Checking application updates on Winget Repository...
-> Available update : Cryptomator. Current version : <IP_ADDRESS>59. Available version : 1.6.14.
-> Available update : BCUninstaller. Current version : <IP_ADDRESS>. Available version : <IP_ADDRESS>.
-> Available update : Node.js. Current version : 16.15.0. Available version : 16.17.0.
-> Available update : Cryptomator. Current version : 1.6.10. Available version : 1.6.14.
-> Available update : Git. Current version : 2.36.1. Available version : 2.37.3.
-> Available update : SWI-Prolog. Current version : 8.4.2. Available version : 8.4.3.
-> Available update : Microsoft Edge. Current version : 93.0.961.38. Available version : 105.0.1343.33.
-> Available update : GNU Privacy Guard. Current version : 2.3.6. Available version : 2.3.7.
-> Available update : WinFsp 2022. Current version : 1.10.22006. Available version : 1.11.22176.
19:16:50 - Cryptomator : Skipped upgrade because it is not in the included app list
19:16:50 - BCUninstaller : Skipped upgrade because it is not in the included app list
19:16:50 - Node.js : Skipped upgrade because it is not in the included app list
19:16:50 - Cryptomator : Skipped upgrade because it is not in the included app list
19:16:50 - Git : Skipped upgrade because it is not in the included app list
19:16:50 - SWI-Prolog : Skipped upgrade because it is not in the included app list
19:16:50 - Microsoft Edge : Skipped upgrade because it is not in the included app list
19:16:50 - GNU Privacy Guard : Skipped upgrade because it is not in the included app list
19:16:50 - WinFsp 2022 : Skipped upgrade because it is not in the included app list
19:16:50 - No new update.
19:16:50 - End of process!

# file: included_apps.txt
OpenWhisperSystems.Signal
BleachBit.BleachBit
BraveSoftware.BraveBrowser
Git.Git
GnuPG.GnuPG
MicroSIP
Docker.DockerDesktop
SpotifyAB.SpotifyMusic_zpdnekdrzrea0
RARLab.WinRAR
WireGuard.WireGuard
Oracle.VirtualBox
DBBrowserForSQLite.DBBrowserForSQLite
Cryptomator.Cryptomator
WinFsp.WinFsp
Microsoft.VisualStudioCode
KeePassXCTeam.KeePassXC
Duplicati.Duplicati
VideoLAN.VLC
Klocman.BulkCrapUninstaller

If I mark the content in your included_apps.txt above there's a lot of spaces behind the names...
...tested a little and the spaces is treated as part of the names.
Delete the spaces (one could TRIM in PS as a security measure...)

Yeah that was the problem. Thanks a lot 👍🏻

Oh, yes. we need to secure that a bit. Thanks guys

#169


ATV-51 | Add audit logging

Description :sparkles:
Add audit logging mechanism. AuditLoggingModelViewSet is included to be able to log all the actions done through a viewset. Viewset calls a separate logging function which writes audit logs in a specified format into the DB. Failing to write audit log will fail the whole operation i.e. action fails if audit log cannot be written. Later logs will be sent from the DB to Elasticsearch. Management commands for sending logs to Elasticsearch and to clean sent logs are included. Management command can be called e.g. periodically from a cron.
Implementation is influenced by earlier work done for appartment-application-service, yjdh, open-city-profile and youth-membership projects.
Log format follows the audit log specification for the project:

https://helsinkisolutionoffice.atlassian.net/wiki/spaces/KAN/pages/6400704679/Audit+log+Asiointitietovaranto
https://helsinkisolutionoffice.atlassian.net/wiki/spaces/KAN/pages/416972828/Helsinki+profile+audit+logging

Codecov Report

Merging #18 (13dbee7) into main (ad91cab) will decrease coverage by 2.43%.
The diff coverage is 76.47%.

@@            Coverage Diff             @@
##             main      #18      +/-   ##
==========================================
- Coverage   85.18%   82.75%   -2.44%
==========================================
  Files          27       36       +9
  Lines         513      719     +206
  Branches       39       55      +16
==========================================
+ Hits          437      595     +158
- Misses         57      101      +44
- Partials       19       23       +4

Impacted Files
Coverage Δ

audit_log/viewsets.py
46.37% <46.37%> (ø)

audit_log/tasks.py
70.96% <70.96%> (ø)

audit_log/audit_logging.py
94.28% <94.28%> (ø)

atv/settings.py
79.22% <100.00%> (+2.40%)
:arrow_up:

audit_log/admin.py
100.00% <100.00%> (ø)

audit_log/apps.py
100.00% <100.00%> (ø)

audit_log/enums.py
100.00% <100.00%> (ø)

audit_log/management/commands/clear_audit_logs.py
100.00% <100.00%> (ø)

audit_log/management/commands/send_audit_logs.py
100.00% <100.00%> (ø)

audit_log/models.py
100.00% <100.00%> (ø)

... and 11 more

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 ad91cab...13dbee7. Read the comment docs.

Failing to write audit log will fail the whole operation i.e. action fails if audit log cannot be written.
Good function


Accessibility add link name to icons

resolve https://github.com/open-cluster-management-io/open-cluster-management-io.github.io/issues/351

/lgtm


Changing Line Height

Hello,
I was searching to set height of each line but could not found much..
Found two related topics
https://stackoverflow.com/questions/39446610/change-line-height-in-avalonedit-5-x
https://github.com/icsharpcode/AvalonEdit/issues/233
Is there any inbuild option for this in 5.0.4?

Unfortunately, no. Sorry, I was wrong when writing my last answer. This is not possible, because we derive the text height from the font.
See https://github.com/icsharpcode/AvalonEdit/blob/8090219cd6407e34c691d1351114e94dea2f23f5/ICSharpCode.AvalonEdit/Rendering/TextView.cs#L1554-L1578


[Bug]: Colspan doesn't work properly

Contact Details
No response
What happened?
Images below without and with colspan property.

Currency column set to "colspan": 5 and the five next column within that row are declared as below with different names each
{
  "name": "Spacer4",
  "type": "span",
  "header": "hide",
  "text": ""
},

What type of foundry hosting were you using?
Local Instance
Relevant console log output
No response

I'll investigate. ColSpan was added ages ago and never documented or really used. Sort of a secret feature. And its implementation and use are a bit different than rowspans too.

I took a quick glance at the code this morning and yeah colspan was never documented because it wasn't used in any official capacity. I think the original intent was to allow for headers to colspan but not data cells. I'd have to go through my chat logs with Emo to really know what was going on at the time.
For now, I'm going to close this issue and re-create as a feature request.  Unfortunately I'm not exactly sure when I'll be able to focus on it, it might be a bit complicated to introduce.


shape file coordinates mismatch

Sorry for this probably silly question. I am a totally beginner.
Why do i get a mismatch in the shape file coordinates I download from geojson.io tool and the coordinates on the right column?
Am I misinterpreting the tool?
If i load the shape file in GE, it goes in place, whereas the coordinate list brings me miles away.
Thanks a lot

whereas the coordinate list brings me miles away.

What does this mean?  Where/how are you using a coordinate list, and what is bringing you miles away?  My suspicion is that you're reversing latitude and longitude in whatever you're testing these coordinates in.
Closing as we can assume this is not an issue with geojson.io.  Happy to help you figure this out if you can provide more context.


Game broken

When attempting to access this at https://githubschool.github.io/github-games-rd-samira-keshavarz/, I am getting a 404. This could be caused by a couple things:

GitHub pages needs to be enabled on master. You can fix this in the repository settings.
the index.html file is incorrectly named inde.html. We will fix this together in class.

Can you please fix the first bullet @rd-samira-keshavarz?

I am not able to see settings in the menu bar. I might have not the access rights.


Bad pivot key name used on BelongsToMany relation

On belongs to many relation, a bad pivot key name is used to find the good relation
https://github.com/Askedio/laravel5-soft-cascade/blob/d0424664f3f1e0b9a0c823cf1d2cf6bd00884e6d/src/SoftCascade.php#L146
Changing getQualifiedForeignPivotKeyName with getRelatedPivotKeyName works for me :)

Hello @squiaios
Can you post the related models to check your error?
Thank you

I have no PHP / eloquent error
Class Foo
Foo.txt
Class Bar
Bar.txt
Migrations
2018_04_15_000000_create_foos_table.txt
2018_04_16_000000_create_bars_table.txt
2018_04_17_160000_create_bar_foo_table.txt
Seeder
FooBarSeeder.txt
Tinker
App\Models\Foo::find(16)->delete();

Yes, I will do it.
I will check package maintenance to follow laravel version standards.

oh perfect, thank you =D

@gcphost Can you create or add me roles for create:
5.5 branch with 5.5.13 version code
5.6 branch with master branch code
With this branches I will make package maintenance to follow laravel versions lifecycle.

Permissions updated @maguilar92 , you should be able to do this now.

@gcphost I can't still create branches.

Weird, I thought as an admin you'd get to do that.
I've created both branches. Let me know if I've done something wrong.

@squiaios Available now at version 5.5.14.
You must change dependency name in composer.json from askedio/laravel5-soft-cascade to askedio/laravel-soft-cascade

perfect, thank you :)


[Bug] Crash when selecting disconnected rail secments

The mod Rail Deconstruction Planner (1.0.2) caused a non-recoverable error.
Please report this error to the mod author.

Error while running event rail-decon-planner::on_player_selected_area (ID 51)
LuaEntity API call when LuaEntity was invalid.
stack traceback:
	[C]: in function '__index'
	__rail-decon-planner__/control.lua:241: in function 'mark_segments'
	__rail-decon-planner__/control.lua:333: in function <__rail-decon-planner__/control.lua:330>

Invalid entity? Interesting. Do you maybe have something that would instantly delete things marked for deconstruction?
I added checks that should catch any cases of other mods invalidating entities while I still expected them to be valid in<PHONE_NUMBER>1d3de2b54de6e92db8f1219224a044 (released in 1.0.3).
Thanks again for reporting this!

No problem. this mod is one of the QOL mods that i would hate to miss out on.
would it be possible to make it work with ghost also? or mixed ghost/real sections? i know there are no options for deconstruction ghosts but you could just remove them right?

The issue is that the functionality requires the rail to be actual rail entities. These provide the functions to retrieve the segments used to traverse the rail.
To support the same for ghost rails I'd need to reimplement all of the connection searches by doing position based entity searches (not even speaking of signals & stations).
A way around this could be to search for ghost rails/signals/stations in a radius around a entity and turn those into "real" ones to reuse the existing traversal.
Then afterwards turn all non removed rails/signals/stations back to ghosts and remove all others.
It's really hacky and other mods could interfere at any point since these conversions always immediately trigger events for all mods and they could do things and mess around (which caused this issue probably).
I'll think about it, but before that I should probably add per-player settings to configure the tool a bit more (remove stations on/off, remove signals on/off (?), remove specific tile below tracks off/tile-select/all(?)). And probably more important a way to just mark a single block instead of the sometimes very far traversal (probably by doing reverse selections by holding rightclick instead of leftclick).


Need some kind of security for sensitive aplications.

I will start by saying that its really nice code, and it saved some work for me and my esp32. Although for some sensitive application it would be better use mqtts instead of mqtt. Because anyone can reverse engineer unencrypted wifi traffic and then inject their own canbus messages which can cause damages, which would be major risk to use this code.
I know that esp32/s3 is capable and Ive seen mqtts examples. I was wondering will you release mqtts version of code? Although it might be slower and will need some code optimization, but I would apreciate this update and make a donation.
P.S. user and password authentication doesnt protect from wifi attacks. Best way would be use encrypted protocol like mqtts.

Try latest.

I will try it out monday, measure speed, and let you know how it goes overall. Thanks, Ive thought you need add extra code to use mqtts

i added a little bit of code.

Yeah I see now that you updated the code aswell. Great job man. You resolved issue very fast.

ESP32C6 has 2 channels of TWAI.
However, ESP-IDF does not yet support a second TWAI.


Why '[object String]'?

Hi!
What's the trivia behind is.string being toString.call(value) === '[object String]' and not typeof value === 'string'?

Hi @avalanche1!
You can have new String("foo").


FINERACT-984-7: Extend GitHub Action pipeline for PostgreSQL

Description
Describe the changes made and why they were made.
Ignore if these details are present on the associated Apache Fineract JIRA ticket.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!

[x] Write the commit message as per https://github.com/apache/fineract/#pull-requests

[x] Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.

[x] Create/update unit or integration tests for verifying the changes made.

[x] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.

[x] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm with details of any API changes

[x] Submission is not a "code dump".  (Large changes can be made "in repository" via a branch.  Ask on the developer mailing list for guidance, if required.)

FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.

Replaced by https://github.com/apache/fineract/pull/2050


EXC_BAD_ACCESS in decodeMessage(decoder:) due to stack overflow

Note: This is likely a duplicate of https://github.com/apple/swift-protobuf/issues/1014 but I didn't want to cram all of this into a comment in a long chain. Feel free to close as duplicate if you want.
Summary/TLDR
The change to move computed properties backed by _storage to properties stored directly on the message structs has caused massive size increases for messages at the top of a deep hierarchy.  The switch code generated for oneof parsing is sensitive to these size increases, leading to massive stack frames.  In debug mode without optimization these stack frames can easily exhaust the 512KB stack of a background dispatch thread and cause a crash.
Details
We are experiencing a crash in debug builds after upgrading SwiftProtobuf and protoc-gen-swift from 1.7 to 1.10.2. Our crash happens in the decodeMessage(decoder:) method of a message containing a oneof with lots of cases.  From what I can tell the stack frame generated for the withExtendedLifetime closure in that method is huge, about 800KB. Since the decoding is happening on a background thread with a 512KB stack, the app crashes as soon as this closure is invoked.
The message is of the form:
message RenderDataDTO {
  SomeMessage1 m1 = 1;
  SomeMessage2 m2 = 2;
  SomeMessage3 m3 = 3;
  oneof render {
    SomeCaseMessage1 case1 = 100;
    SomeCaseMessage2 case2 = 101;
    SomeCaseMessage3 case3 = 102;
    // 20 cases in total
    SomeCaseMessage20 case20 = 120;
  }

The code that gets generated for decodeMessage(decoder:) has the form:
  public mutating func decodeMessage<D: SwiftProtobuf.Decoder>(decoder: inout D) throws {
    _ = _uniqueStorage()    // LINE A
    try withExtendedLifetime(_storage) { (_storage: _StorageClass) in    // LINE B
      while let fieldNumber = try decoder.nextFieldNumber() {
        switch fieldNumber {
        case 1: try decoder.decodeSingularMessageField(value: &_storage._m1)
        case 2: try decoder.decodeSingularMessageField(value: &_storage._m2)
        case 3: try decoder.decodeSingularStringField(value: &_storage._m3)
        case 100:
          var v: SomeCaseMessage1?
          if let current = _storage._render {
            try decoder.handleConflictingOneOf()
            if case .case1(let m) = current {v = m}
          }
          try decoder.decodeSingularMessageField(value: &v)
          if let v = v {_storage._case1 = .case1(v)}
        case 101:
          var v: SomeCaseMessage2?
          if let current = _storage._render {
            try decoder.handleConflictingOneOf()
            if case .case2(let m) = current {v = m}
          }
          try decoder.decodeSingularMessageField(value: &v)
          if let v = v {_storage._case2 = .case2(v)}
       // etc...

If I set a breakpoint at the line marked LINE A and check the stack pointer, then set a breakpoint at LINE B and do the same I find that the difference is about 800KB.
Here's one example:
RenderDataDTO.decodeMessage(decoder:)
       rsp = 0x00007000087cdb60

In closure:
       rsp = 0x0000700008704780   <-- 800KB jump!

I have a couple of observations to explain this:
Observation 1:
In the earlier version of swift-protobuf almost all fields of Message structs were stored in _storage, a reference-typed container, and exposed as computed vars. In the current version they seem to much more often (but not always?) be implemented as properties on the message itself.  Thus the size of a leaf message struct with no child messages became O(n) instead of O(1), where n is the number of fields in the message.
The story gets a lot worse when you have child messages. Since messages are structs, children get embedded directly in their parents, so the size of a non-leaf message grows like the sum of its entire message sub-tree.  So the size scales more like O(n^m), where n is the average number of children per level and m is the average number of levels in the message hierarchy.  Since Swift always likes to allocate structs on the stack, this can lead to massive stack frames.
This agrees with what I saw when I measured the size of the SomeCaseMessageN messages using MemoryLayout<SomeCaseMessageN>.size. In the older code, they were all 24 bytes, with a few exceptions.  The largest was 80 bytes.  In the new code the sizes tended to increase by 10X-20X.  224 and 448 bytes were typical sizes, with several messages growing above 1000B.  One grew to 5256 bytes, while the biggest was 6080 bytes.
Observation 2:
If the compiler is smart, it will know that all the cases in the switch statement inside that closure are mutually independent. Only one of them will ever execute per invocation, so their local variables can share space on the stack, and the size of the stack frame will be the size of the biggest case in the switch.  Since we're in debug mode it might not be making that optimization, and instead allocating separate space for every case in the switch.  Given the size impact of the changes I discussed above, this could add up to a lot of space.
In fact, when I change the optimization mode from none to size or speed, the message sizes don't change but the size of the stack frame drops to about 26KB. That's still an oversized stack frame eating 5% of the stack in release mode, but at least it's not bigger than the entire stack!
For reference, with the code generated from 1.7, the stack frame is about 16KB with optimization mode none.  With optimization mode speed the stack frame drops to just 400B.
Wrapping up
There seem to be problems with putting so many stored properties on messages. Perhaps this could be dialed back to where it was before?
Environment:
OS: MacOS Catalina 10.15.6
Xcode: Version 11.6
Swift 5

I'm guessing this is the PR that led to this problem: https://github.com/apple/swift-protobuf/pull/900

Hi, author of #900 here. This doesn’t look like a particularly fun bug—thanks for the diagnosis. The rationale for #900 was to reduce ARC / heap pressure, primarily for use cases involving lots of small messages.
In other languages, message fields are typically stored by reference, rather than by value (an Optional in Swift being just that, not syntactic sugar on a pointer), which makes passing deeply-nested messages cheap.

I’m curious if when this package was originally developed, were class-based messages (versus structs) considered?
The underlying Optional for message fields could theoretically be replaced with a reference-based storage class (rather than storing all fields in a storage class).
Could you share the proto that results in a 6KB message?
It sounds like you preferred the previous reference (versus value) behavior for message fields. Are you copying trees of messages?
Last, the stack growth for the oneof switch statement seems fixable.

Hi, thanks for the response! To answer your questions:

I wasn't around when this app was created so I don't know what was considered. Are class-based messages an option? I would expect them to be a better fit for our use case.
Using a reference for message fields would help a lot, and keep the size of messages linearly proportional to the number of fields.  My thought (based on the conversation in https://github.com/apple/swift-protobuf/issues/735) would be to use value-based storage while MemoryLayout<MessageType>.size <= 24, then switch to reference-based storage.  If it's too hard to find a solution that works for everybody, maybe offer an option to choose the storage strategy.
I can't share the proto files publicly. The message tree represents the data and metadata of a complex interactive data visualization of the type seen here.  Here's an illegible screenshot of the message subtree rooted at that particular message:

As you can see, the hierarchy is very deep, and this is only one subtree.
Yes, and yes. 😄  Value types are great for small flat chunks of data, but terrible for tree-structured data without indirection. The current SwiftProtobuf is unusable for us.
Does It? What do you have in mind?

Given that the encoder now assumes the protobufs will be value types, we could take a step to give some flexibility here by potentially modifying the codegen. There are a lot of use-cases where the reduced allocator pressure and improved data locality of simple structs is really valuable, and I’d like to find a way to keep it.
So, what if we considered having a codegen option that would change this behaviour to instead generate class-backed structs: that is, structs whose only stored member is a class, and that use entirely computed properties to compute their fields. These fields would still be stored properties on the class, giving good performance to load, but the structs would no longer be nested, so they’ll be scattered all over the heap as if they were classes. This will keep their sizes down.

If the stack usage in the generated function is not a Swift compiler bug, then it might make sense to actually count oneof fields while computing useHeapStorage in the MessageGenerator.
If that isn't sufficient then it seems like weighting fields by their size in the useHeapStorage calculation makes sense too, as suggested.
Either way I think understanding why the function is using so much stack is a great idea. Perhaps we could do better just in how we lay out the code, like by using nested functions.
@n8gray , out of curiosity, if you add a field of type "Tree Root Message" to the declaration of the problematic message (and obviously never set it), does the generated code then work? This will trigger useHeapStorage because of the declaration cycle.
I am not in favor of adding an option for this. I mean, just pondering the question of how you would document it reveals that it wouldn't be clear or usable.

I’m curious if when this package was originally developed, were class-based messages (versus structs) considered?

ObjC protos are the reference model, and it causes no end to folks getting threading wrong.  Especially with a deep chain like you have. Folks have to remember once they set a message into a field they can't touch it again since the message is referencing it.  But if the messages are used for models or to collect things and send them off for logging, it is way to easy to accidentally end up making changes, setting as a field on a message to send to a server and then continuing to mutate it for the next thing to send out.  Even if you try to copy the message on field setting (which greatly slows things down), there are still potential issues with repeated fields and map fields since you can't force a copy on the way into those structures without rolling your own custom ones.  So value times makes it things much harder to have threading issues.
The current calculation for when to use storage has always been a bit of a minimal guess and it too shallow.  I've got some ideas I've toyed with in the past to start computing real costs of objects to better decided when to transition to storage which should help the general problem here.  But all of those past thoughts have been purely around the stack/heap costs and not looked specifically to much at the exact cost of the oneof enums but definitely not thought about the cost on extended lifetime calls.
We should be able to adapt to a better computed cost as a trade off between the old and current approaches, which will hopefully solve this specific problem, and it can always continue to be tuned in the future.  I'll try to take a look at this early next week.
@Lukasa the Storage model already is a class backed storage.  As mentioned on the FAQ, options that impact how things generate are rarely a good path because inevitable you end up with someone having a dependency on protos generated one way, and someone else depending on the same message(s), but wanted to generate it with another set of options.

If the compiler is smart, it will know that all the cases in the switch statement inside that closure are mutually independent. Only one of them will ever execute per invocation, so their local variables can share space on the stack, and the size of the stack frame will be the size of the biggest case in the switch. Since we're in debug mode it might not be making that optimization, and instead allocating separate space for every case in the switch.

This is exactly what's happening—SwiftSyntax had the same issue with its visitation methods for syntax trees because they contained very large switch statements that created instances of value types in the cases. They solved it by moving each individual case into its own function.
I don't think we need to do something that drastic here. There's a much easier change that I think we can make to reduce stack usage for debug builds—we just update the generator so that we nest the body of each case in an immediately executed closure:
      switch fieldNumber {
        case 1: try { try decoder.decodeSingularMessageField(value: &_storage._m1) }()
        case 2: try { try decoder.decodeSingularMessageField(value: &_storage._m2) }()
        case 3: try { try decoder.decodeSingularStringField(value: &_storage._m3) }()
        case 100: try {
          var v: SomeCaseMessage1?
          if let current = _storage._render {
            try decoder.handleConflictingOneOf()
            if case .case1(let m) = current {v = m}
          }
          try decoder.decodeSingularMessageField(value: &v)
          if let v = v {_storage._case1 = .case1(v)}
        }()
        ...

(I'm probably missing some explicit selfs that would be required by doing that; SE-0269 may reduce them but we'd have to adopt a Swift 5.3 minimum to rely on that.)
Even though the oneof cases like case 100 are the ones using obvious local stack storage, I believe even the others will use some as intermediate storage (at least in an unoptimized build) to handle the retrieval and modification of the _storage fields, so the surest way to reduce the stack usage is to wrap all of them.
The double-try requirement (one for the throwing operation inside the closure, and then again around the outer closure) is unfortunate, but I'm not sure there's a way to avoid it.
This will result in an extra function call for debug builds, but optimized builds should remove the closure completely. But by moving each case into its own "function", we ensure that it only occupies space on the stack when that case is executed, instead of the sum of all the cases being on the stack for the entire execution of the outer function.
There are other improvements we should look into to remove the likelihood that the memory size of messages is ever excessively and surprisingly large, but in the meantime, the above should be a straightforward change that would still be useful independent of other improvements.

@allevato Brilliant! That sounds like a great solution to the stack frame problem.
@thomasvl It’s good to hear you’re willing to take this on! I agree it’s best to avoid knobs and switches when possible, so I hope you can find heuristics that strike the right balance and avoid these problems with deep hierarchies.

@cburrows I’d try out your idea for adding a recursive reference but I’ve reverted all my pods and brews back to 1.7 at this point.  I’m sure it would succeed at forcing that message to use heap allocation, but there are many other messages in our app that will have the same problem.

I can clarify the thinking behind the current design a bit.
First of all, class vs. struct is a somewhat over-simplified way to think about this.  The two questions that really matter are:

value semantics vs. reference semantics ?  and
inline storage vs. indirect storage?

The first question involves what happens when you copy and then mutate a value:  Reference semantics cause all copies to reflect the mutations; value semantics mean that mutations do not affect other copies.  (In particular, as @thomasvl pointed out, value semantics have real advantages for multi-threaded code.)  The latter question involves how the storage gets actually allocated.  Traditional Obj-C style classes provide reference semantics and indirect storage.  Naive use of Swift structs give you value semantics and inline storage.  But value semantics are not incompatible with indirect storage; indeed much of the standard library (String, Array, etc) provides value semantics with indirect storage by using a struct or enum as a facade for a separate heap-allocated data store.
So to answer the real questions:

SwiftProtobuf has always provided value semantics for the generated types.  This was a very deliberate decision.
Internally, SwiftProtobuf has support logic for both inline and indirect storage, but this is always hidden from the user.  (In particular, any generated _Storage class is kept private.)  Indeed, it should be possible to store some fields inline (directly in the struct facade) and some indirectly (in the _Storage class) for a single generated message.

The difficult piece has always been deciding what fields should be stored inline and what should be stored indirectly.  There is really only one hard requirement:  reference cycles cannot be stored entirely inline.  But otherwise, we have a lot of options to consider:

Up until version 1.7, SwiftProtobuf used a simple heuristic:  If this message had more than 16 fields or if it had singular sub-messages, then everything would be stored indirectly (in the generated _Storage class).  Otherwise, everything was stored inline.
Issues #733 and #735 pointed out a significant shortcoming:  This caused excessive memory usage for small messages that happened to be split out into submessages.  In particular, this penalizes the well-known wrapper types.
PR #900 changed the logic to try to minimize the use of indirect storage.
This Issue #1034 and the related #1014 seem to suggest that PR #900 went a bit too far.

I'm not eager to provide options to control the layout.  I think we might have to do that someday, but I'd rather focus our attention now on finding better heuristics so that most users can get good results without having to manually tweak a lot of options.
Here are some ideas I would like to explore:

For oneof fields, we could consider making the generated enum be indirect if any case had a message type.
We could store some fields inline and some in the backing _Storage class.  This would require some work on the generator to allow storage and accessor differentiation on a per-field basis.
Once we are able to make per-field storage decisions, we have a lot of choices:  Protobuf encoding is designed to be more efficient for fields numbered <= 15, so it might make sense to prefer inline storage for those; we could store submessages inline if we knew they were "simple enough";  we could estimate the in-memory size of the fields and use that to guide decisions; many other possibilities come to mind.

@n8gray -- It looks like your message hierarchy might be entirely tree-structured, which raises an interesting question:  Would it make more sense for each sub message to be a separate heap allocation?  Or would it be better for the top-level message to be allocated on the heap as a single 6k structure with the submessages inlined into that storage?

@tbkka Thanks for laying out the history and trade offs in a clear and concise way. As I mentioned earlier, the protobufs in our app represent the structure and data of complex interactive data visualizations, so they are large, deep trees.  The root of the tree is usually just some FooRequest or FooResponse, with subtrees for data, metadata, filters, settings, contexts, etc.  This is, I’d say, the diametric opposite of the use case in #733 where they were working with massive volumes of small messages. Instead, we have one giant message that is split apart for use in multiple subsystems. As the app is developed, new message cases, levels, and entire subtrees are constantly being added and removed by a team of dozens of developers.
With that in mind, the answer to your question is that pushing the inlining down one level is not going to help, as the next level will likely be just as bad, and if it’s not today it could easily be tomorrow. (In fact the “Root object” we are discussing is not top level at all, it just happened to be at a level that had a lot of oneof cases and triggered the stack problem.)
Aggressive inline storage is never going to work for our use case as a general strategy. It’s a great optimization to prevent extra boxing at the leaves of the tree, and to get cost-free abstraction when you want to wrap a single item or two in a message for whatever reason, but that’s probably it. We need a solution that reliably scales to lots of fields, lots of depth, and lots of oneof cases.  Indirect storage is reliable that way, at the expense of performance penalties at the leaves of the tree.  Fine for us, not great for #733.
The more I think about it the harder it seems to find a strategy that will work for both. Hopefully you folks have more imagination. 😉

Catching back up on the comments -
The switch mentioned is in decoding, but we also generate switch statements for some of the oneof handling: required field validation, generating equality code, and traverse/visit. Will they have the same stack related issues when the optimizer is off (and if the message has enough large message fields in the oneof)?  Those all also generate case xxx(let yyy) in the switch, so would the closure trick work there since the let would be outside the closure scope?

All switch statements would be affected, so if a branch is likely to involve a large message, it could hit the same problem.
In general, a function's stack usage in debug builds will grow proportionally to the sum of the local storage of the cases in its switch statements, but this also affects if/else if... branch structures as well. My completely uneducated guess (i.e., not going to look at IRGen before speaking) is that debug builds just give each basic block in the generated code its own space for locals, even when execution is mutually exclusive.
Binding an associated value in a case makes things a little trickier, but we can still work around it. It's ugly though. Instead of this:
switch foo {
  case .bar(let value): try doSomething(with: value)
}

we'd do this:
switch foo {
  case .bar: try {
    guard case .bar(let value) = foo else { preconditionFailure() }
    try doSomething(with: value)
  }()
}

Using preconditionFailure() here instead of fatalError() is important; the former will get removed in optimized builds (making this the same as the original code, at least in the simple tests I tried), but the latter wouldn't be.
If we go and make any of these changes, we should consider them carefully—we're giving the compiler/optimizer more work to do, for example, so we need to make sure that by reducing stack usage we're not raising compile time (for both types of builds), especially for targets with large messages.

@n8gray Thanks for clarifying.

... pushing the inlining down one level is not going to help, as the next level will likely be just as bad, and if it’s not today it could easily be tomorrow ...

I wasn't really proposing pushing the inlining down exactly one level -- there's no really practical mechanism for determining what that means -- but rather trying to better understand whether your concern was about a few large top-level messages or a larger number of intermediate messages.  As you pointed out, inlining still looks like a good choice for small messages, and #733 is really about how we can avoid unnecessary boxing for very small messages.  (~32 byes in that case)

... expense of performance penalties at the leaves of the tree ...

  What would happen in your use case if messages smaller than about 64 bytes were stored inline?  (Allocating a single object on the heap has about that much overhead by the time you add up the Swift object header, malloc data structures, and allocation rounding.)

That almost seems like it might rely on another compiler bug then. If the body of the guard...else becomes empty, then how is the return on the else satisfied? Shouldn't the compiler catch that in some optimization modes, the guard...else is invalid because it doesn't return?

So I was being a little loose with terminology above. Here's the behavior:
fatalError unconditionally prints a message and then unavoidably traps (Builtin.int_trap). The optimizer won't remove this, no matter what.
preconditionFailure only prints a message in debug mode. Then, in both debug and release mode, it invokes a special Builtin.conditionallyUnreachable, which is treated differently than the trap above. In this case, the optimizer sees that the code inside the guard ... else block is unreachable so it can drop the condition entirely and just bind the associated value directly.

@tbkka

How big are your leaf messages? What would happen in your use case if messages smaller than about 64 bytes were stored inline? (Allocating a single object on the heap has about that much overhead by the time you add up the Swift object header, malloc data structures, and allocation rounding.)

It's hard to generalize since we have over 100 .proto files, but I wouldn't expect a 64 byte limit to present major issues. We're probably building structs in our own code that are around that size.  I honestly don't think our app is all that sensitive to the absolute performance of allocating protobufs.  We basically make one big request when you enter a visualization and then some smaller requests when you select different data points, but I'm sure the network latency dwarfs the (de)serialization costs.  My main performance concern would be making sure whatever inline storage limit gets set is respected no matter how deep or wide the tree gets, so there aren't sneaky edge cases where you end up with 1KB structs.  Hard-to-reproduce stack overflow crashes are way more frightening than an extra 20ms of parsing time or an extra MB of heap overhead.

Also, re this being a duplicate of #1014, I don't believe so.  #1014 is crashing the some of the Swift initialization code.  Yes, the amount of stack using during decode also plays into that, but it is always going to be possible for that Swift internal runtime code to need more space than what is available in some context. Fixing the issues here won't guarantee that some other case couldn't still fail in the same way #1014 fails.

Some more quick findings:
Having a try in the body of the case still results in a small amount of local stack usage for that case, so we can't eliminate the local growth completely if we want to propagate error state out:
      switch fieldNumber {
        case 1: try { try decoder.decodeSingularMessageField(value: &_storage._m1) }()
        case 2: try { try decoder.decodeSingularMessageField(value: &_storage._m2) }()
        case 3: try { try decoder.decodeSingularStringField(value: &_storage._m3) }()

The outer trys on each of these still uses a small amount of space. I tried a few things, like wrapping them in Results that got immediately assigned to some variable outside the switch, but it looks like the codegen still assigns some temporary space inside the basic block (and more than try would have used) that doesn't get eliminated until the optimizer passes run.
Whether the message uses a storage class or not also makes a difference. If we pass a stored property of a struct to a function using inout, it looks like even in debug mode, no extra stack space will be used to read/modify/write that value (at least in the simple tests I tried). If we change it to a class instead, then stack space is used (and significantly more), likely due to all the extra retains/releases inside each branch, but also space is used to read/modify/write the value instead of doing it in-place. Fortunately we're already doing the right thing to eliminate the retains/releases by calling withExtendedLifetime.
So if we go the direction of wrapping each case in a closure, we could possibly avoid the closures for fields like cases 1, 2, 3 in messages that don't use a storage class, but we have to ask whether saving that extra call instruction and prolog/epilog are worth the added complexity in the generator.

First PR up.  In theory, we could attempt to compute the cost of each field and only need to use this closure workaround when things are over some size check, but given a message or message chain can be recursive, there's no way to tell how expensive those would be.  So those cases would still have to use closures for everything, so it seems better to just keep the generator simple and if we generate a closure, always generate a closure.  i.e. - less variation in the generated code, less to prove correct.

@n8gray if you can test with master, I'd be curious how much the just merged changes help your situation.  I'll work on when Storage is used next, but the first data point on your real world case would also be useful.

Thanks, I'll try to check it out today.

@thomasvl The new codegen has solved the stack issue for decodeMessage. The stack frame is now 736 bytes in debug builds. Nice work! 👏

Second round of work which makes the calculation around use of Storage transitive (and thus consistent), is now on HEAD.  That should reduce the usage a little more.
We'll let this sit on HEAD for a few days before considering cutting a new release to get these changes out to everyone.
While there are other place we likely could do more codegen changes (the if cascades), I'm going to call this fixed and if folks find other concrete cases where things fail, we'll go after them in new issues.

I just did some testing on 6806de79a49cb2b81155b8b431af6bf9be1d4af9 and it looks a lot better. The sizes of the message structs I mentioned before have dropped significantly:
Size: 6080 -> 24
Size: 5256 -> 160
Size: 1944 -> 24
Size: 1400 -> 24
Size: 1096 -> 24
Size: 704 -> 144
Size: 456 -> 144
Size: 448 -> 72
Size: 440 -> 24
Size: 432 -> 24
Size: 256 -> 256
Size: 224 -> 224
Size: 224 -> 224
Size: 208 -> 208
Size: 136 -> 136
Size: 128 -> 128
Size: 88 -> 88
Size: 80 -> 80
Size: 80 -> 80
Size: 48 -> 48
Size: 48 -> 48

I still have misgivings about letting structs grow to 256 bytes but the worst case seems to be under control. Thanks!

Yea, as mentioned on the other issue and the cl, we've not got a framework in place to actually use real costs going forward which should be much better.


Bump cryptography

Before submitting this PR

Description: Add a summary of the changes in this PR and the related issue.
Jira task / GitHub issue: Link to the github issue or add the Jira task ID here.
How to test: Add information on how someone could manually test this functionality. As detailed as possible.
Type of change: Check the relevant boxes in the section below
Add docstrings and comments to code, even if you personally think it's obvious.

What type of change(s) does the PR contain?

[ ] New feature

[ ] Breaking: Please describe the reason for the break and how we can fix it.
[ ] Non-breaking

[ ] Bug fix

[ ] Breaking: Please describe the reason for the break and how we can fix it.
[ ] Non-breaking

[x] Security Alert fix
[ ] Documentation
[ ] Tests (only)
[ ] Workflow

Checklist

Sprintlog

[x] Added
[ ] Not needed (E.g. PR contains only tests)

Rebase / Update / Merge from base branch (the branch from which the current is forked)

[ ] Done
[ ] Not needed

Blocking PRs

[ ] Merged
[x] No blocking PRs

PR to master branch

[ ] Yes: Read the release instructions

[ ] I have followed steps 1-7.

[x] No

Actions / Scans

Black: Python code formatter. Does not execute. Only tests.
Run black . locally to execute formatting.

[x] Passed

Pylint: Python code linter. Does not execute. Only tests.
Fix code producing warnings. Code must get 10/10.

[ ] Warnings fixed
[x] Passed

Prettier: General code formatter. Our use case: MD and yaml mainly.
Run npx prettier --write . locally to execute formatting.

[x] Passed

Yamllint: Linting of yaml files.

[x] Passed

Tests: Pytest to verify that functionality works as expected.

[ ] New tests added
[x] No new tests
[x] Passed

TestPyPi: Build CLI and publish to TestPyPi in order to verify before release.

[x] Passed

CodeQL: Scan for security vulnerabilities, bugs, errors

[ ] New alerts: Go through them and either fix, dismiss och ignore. Add reasoning in items below.
[ ] Alerts fixed: What?
[ ] Alerts ignored / dismissed: Why?
[x] Passed

Trivy: Security scanner

[ ] New alerts: Go through them and either fix, dismiss och ignore. Add reasoning in items below.
[ ] Alerts fixed: What?
[ ] Alerts ignored / dismissed: Why?
[x] Passed

Snyk: Security scanner

[ ] New alerts: Go through them and either fix, dismiss och ignore. Add reasoning in items below.
[ ] Alerts fixed: What?
[ ] Alerts ignored / dismissed: Why?
[x] Passed

@valyo Also ready for review when you have time


FEAT: Uninstall script to remove both working dirs and remove all symlinks

Given no access to the SQLite db
When a user uninstalls the app
Then it should remove both content and settings and remove all symlinks
Note this should cover both happy and unhappy paths where the db can either be intact or corrupt.
The idea is that it shouldn't matter on uninstall.

This is done right?

No this would be the electron uninstaller so it removes everything if a user nukes the install without opening the app.

Resolved in #51


refactor!: poetry, github action overhaul, pyenv / asdf versions

Fixes #136

@jezdez I meant to close this PR.  This was just experimenting ^

I'd rather switch to hatch than poetry to be honest, as it's probably better supported via the PyPA.

hatch, that's totally new to me!

I like seeing the move from setup.py to pyproject.toml though!

I'm happy to contribute that in a separate PR (and even try hatch out)
That said - the bigger issue I'm concerned about is #124.
@jezdez What are your thoughts on this project here and now? Am I doing more harm than good? Do you want to pilot it again? (I notice this specific django package your creation, on atop of your being apart of jazzband).  Can / should I email you?

For now I will close this - per https://github.com/jazzband/django-robots/pull/137#issuecomment-1280049961

@jezdez
Where I took this:

re: hatch, pipenv, poetry, etc: https://github.com/jazzband/help/issues/313

Best practices: https://github.com/jazzband/help/issues/314
in the above I also floated the idea of:

a "flagship" project that can champion the best practices for other projects to imitate
potential sprints to update best practices by hand (e.g. setup.py -> pyproject.toml across all projects)

potential sprints to update best practices by hand (e.g. setup.py -> pyproject.toml across all projects)

fyi Hatch can pretty robustly automate that https://hatch.pypa.io/latest/intro/#existing-project

I did one 🙂 https://github.com/jazzband/django-debug-toolbar/pull/1690


Add labels to scrape cilium agent and operator metrics

The prometheus queries in dashboards provided as part of the cilium examples use the labels k8s_app(in cilium agent) and io_cilium_app(in cilium operator) as metric labels. But the service monitor manifests provided as part of the helm chart don't add these labels as part of scraping the metrics. As part of this PR I've added these labels to the targetLabels of the serviceMonitor manifests. Ref slack conversation

test-me-please

test-me-please

retest-runtime

retest-runtime

retest-net-next

retest-net-next

retest-netnext

retest-netnext


Added a eclipse android.

Added a eclipse android tutorial of build example.

Is this supposed to be a pull request? Or a feature request? I'm not really sure what to do with this "issue". I'm closing this, feel free to edit and reopen it, once clarified.

Please use the forum for feature requests and general discussions.
See also contribution guidelines.


Disable syntax highlighting

First of all thank you for react-syntax-highlighter. We use it in atlaskit library and it's great!
There's an issue which we're still missing in react-syntax-highlighter: disable syntax highlighting. It looks like lowlight.js is also missing this feature while highlight.js supports this through classnames.
I looked into into react-syntax-highlighter code and didn't find an easy way to do this. In our code the easiest way to do this is to do a manual check and mimic react-syntax-highlight HTML layout on our part:

But it would be great to have "text" language support in react-syntax-highlighter

but i've published v5.8.0 which allows passings text as a language.
Thanks for the issue!

@conorhastings hm, it's still missing here: https://github.com/conorhastings/react-syntax-highlighter/blob/master/AVAILABLE_LANGUAGES.MD

@1999 it is I've implemented a more complete solution and pushed a patch . It won't show up here --  https://github.com/conorhastings/react-syntax-highlighter/blob/master/AVAILABLE_LANGUAGES.MD -- because this is a list of importable language syntaxes which text is not, i've made a not in the language section of the readme though. Thanks for the feedback
published v5.8.1 if you want to check that out


Dynamic large file

After several test. While I test with file large than 5.3KiB, it can't be done. Output from ./decode output always 5.3KiB.
Thankyou..

This modem is only meant to transmit short datagrams. For larger files, you need to split the file and transmit each shard individually.

Thanks for the information. I'll try this method in script. 73


ChatQnA: accelerate also teirerank with Gaudi

Description
Accelerate also teirerank with Gaudi, not just tei.
When reranking is used, it does not make sense (performance-wise) to accelerate just tei, as reranking is a larger bottleneck.
Issues
n/a.
Type of change

[x] Bug fix (non-breaking change which fixes an issue)

Dependencies
n/a.
Tests
Manually checked the ChatQnA throughput.

Marking as draft because I'm not sure in which other places this should be added.
I can add common/teirerank/gaudi-values.yaml, but IMHO guardrails-gaudi-values.yaml files should be something added on top of gaudi-values.yaml, not independent.

Looking at the CI fails, CI seems to be currently in rather broken state:

pre-commit.ci fails due to CI check for Python doc formatting not being configured properly
Change to gaudi-values.yaml cannot be cause for the Xeon failures
PR does not touch guardrail files yet either

I've fixed the pre-commit failure.
Guardrail failure should be caused by other changes(Docker or Python), I'm debugging it but maybe we can disable it first to make  the CI work.
Please hold a while until this one merged.https://github.com/opea-project/GenAIInfra/pull/473
Besides the guardrail fail, there is another relative path check failed, I'm sure it's not due to the code itself, but https://github.com/yongfengdu/GenAIInfra/blob/ci-fix/.github/workflows/pr-path-detection.yml

The Xeon failure is caused by https://github.com/opea-project/GenAIExamples/pull/891, which removed the use of microservice layers for LLM and Embedding. #474 is the follow up change for helm-charts.
For the guardrails-gaudi-values.yaml, unfortunately there is no way to include gaudi-values.yaml in helm chart, so it's ok to do duplicate changes there, or just keep it as is(Guardrail case still use CPU for reranking).
You'll have to rebase with latest change to continue.

For the guardrails-gaudi-values.yaml, unfortunately there is no way to include gaudi-values.yaml in helm chart, so it's ok to do duplicate changes there, or just keep it as is(Guardrail case still use CPU for reranking).

I added gaudi-values.yaml to common/teirerank/, but did not touch guardrails, as I haven't done any performance testing for it.

You'll have to rebase with latest change to continue.

Thanks, done!

I dropped the PR draft status, but I haven't tested this yet with the ChatQnA "nowrapper" changes that were merged after I filed this.  I would expect rerank perf to be even more important after its wrapper service is not providing extra buffering / slowdown though...

As Gaudi rerank worked fine for me, CI failure for it:
2024-10-16T13:00:32.007226Z  INFO text_embeddings_backend_python::management: backends/python/src/management.rs:136: Python backend process terminated
Error: Error when doing warmup

Caused by:
    Could not start backend: max_warmup_length (1024) exceeds model's max_input_length (512), you can modify this value adding `-e MAX_WARMUP_SEQUENCE_LENGTH=<new_warmup_length>` to your Docker run command

could be result of the later nowrapper changes. I think it's another bug in CI though, of only specifying one of the 2 related TEI options.

    Could not start backend: max_warmup_length (1024) exceeds model's max_input_length (512), you can modify this value adding `-e MAX_WARMUP_SEQUENCE_LENGTH=<new_warmup_length>` to your Docker run command

I think it's another bug in CI though, of only specifying one of the 2 related TEI options.

This is not a bug in CI, but with git HEAD, max warmup (matching specified max input length) is given only for tei, not teirerank: https://github.com/opea-project/GenAIInfra/issues/483

Rebased teirerank config fix as first one, so that every commit works.
This PR does not change anything related to guardrails, but still that test fails, due to another CI bug:
#kubectl logs chatqna20241018105227-testpod -n chatqna-20241018105227
curl: (22) The requested URL returned error: 500
Internal Server Errorcurl failed with code 22

Fail is due to ChatQnA timeouting on guardrails:
-----------Pod: chatqna20241018105227-6897bff66b-cm9sv---------
#kubectl describe pod chatqna20241018105227-6897bff66b-cm9sv -n chatqna-20241018105227
...
     Image:           <IP_ADDRESS>:5000/opea/chatqna-guardrails:latest
...
INFO:     <IP_ADDRESS>:36050 - "POST /v1/chatqna HTTP/1.1" 500 Internal Server Error
ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/aiohttp/connector.py", line 1091, in _wrap_create_connection
    sock = await aiohappyeyeballs.start_connection(
...
 TimeoutError: [Errno 110] Connect call failed ('<IP_ADDRESS>', 80)
...
 aiohttp.client_exceptions.ClientConnectorError: Cannot connect to host chatqna20241018105227-guardrails-usvc:80 ssl:default [Connect call failed ('<IP_ADDRESS>', 80)]

Although that service gets (eventually) to Ready state and its log shows now errors:
NAME                                                     READY   STATUS      RESTARTS       AGE     IP               NODE                NOMINATED NODE   READINESS GATES
...
chatqna20241018105227-guardrails-usvc-7658584b7c-4d25z   1/1     Running     0              3m22s   <IP_ADDRESS>   aise-cluster-01-3   <none>           <none>
...
-----------Pod: chatqna20241018105227-guardrails-usvc-7658584b7c-4d25z---------
#kubectl describe pod chatqna20241018105227-guardrails-usvc-7658584b7c-4d25z -n chatqna-20241018105227
...
    Liveness:        http-get http://:guardrails-usvc/v1/health_check delay=5s timeout=1s period=5s #success=1 #failure=24
    Readiness:       http-get http://:guardrails-usvc/v1/health_check delay=5s timeout=1s period=5s #success=1 #failure=3
    Startup:         http-get http://:guardrails-usvc/v1/health_check delay=5s timeout=1s period=5s #success=1 #failure=120
...
  Type     Reason     Age    From               Message
  ----     ------     ----   ----               -------
  Normal   Created    3m11s  kubelet            Created container chatqna20241018105227
  Normal   Started    3m11s  kubelet            Started container chatqna20241018105227
  Warning  Unhealthy  3m5s   kubelet            Startup probe failed: Get "http://<IP_ADDRESS>:9090/v1/health_check": context deadline exceeded (Client.Timeout exceeded while awaiting headers)
...
-----------Pod: chatqna20241018105227-guardrails-usvc-7658584b7c-4d25z---------
#kubectl describe pod chatqna20241018105227-guardrails-usvc-7658584b7c-4d25z -n chatqna-20241018105227
...
#kubectl logs chatqna20241018105227-guardrails-usvc-7658584b7c-4d25z -n chatqna-20241018105227
...
INFO:     <IP_ADDRESS>:34224 - "GET /v1/health_check HTTP/1.1" 200 OK
INFO:     <IP_ADDRESS>:50580 - "POST /v1/guardrails HTTP/1.1" 200 OK
INFO:     <IP_ADDRESS>:52736 - "GET /v1/health_check HTTP/1.1" 200 OK

=> CI runs the query before verifying that all necessary backend pods (at least TGI & TEI) have reached Ready state?

=> CI runs the query before verifying that all necessary backend pods (at least TGI & TEI) have reached Ready state?

@lianhao Has this not been fixed yet: https://github.com/opea-project/GenAIInfra/pull/454#issuecomment-2373530751  ?

The CI failure is caused by this commit https://github.com/opea-project/GenAIExamples/pull/977
which changed the default port of guardrails, but we didn't specify the GUARDRAIL_SERVICE_PORT in our helm chart.

=> CI runs the query before verifying that all necessary backend pods (at least TGI & TEI) have reached Ready state?

@lianhao Has this not been fixed in CI yet: #454 (comment) ?

This failures is caused by a recent PR #977 in GenAIExample

let's  wait for PR #489 to land-in first

The CI failure is caused by this commit opea-project/GenAIExamples#977 which changed the default port of guardrails, but we didn't specify the GUARDRAIL_SERVICE_PORT in our helm chart.

Any idea why that regression was not caught by CI?

Because it's a change in GenAIExamples repo, not in this GenAIInfra repo.

Because it's a change in GenAIExamples repo, not in this GenAIInfra repo.

Hm.  Such breakage seems to happen often enough that I think PRs in those repos could trigger (e.g. after merging) also GenAIInfra CI tests...

Because it's a change in GenAIExamples repo, not in this GenAIInfra repo.

Hm. Such breakage seems to happen often enough that I think PRs in those repos could trigger (e.g. after merging) also GenAIInfra CI tests...
EDIT: Or if cross-repo triggers are not possible, maybe there could be automated GenAIInfra CI test runs at some specific interval (e.g. weekly or more often), to catch regressions caused by changes in external dependencies. It would be nice if such tests could file the detected issues as bugs ("[Bug] CI external dependency test 2024 WW43 - failed").

Cross-repo trigger seems not available right now. We have automated trigger to create issue in GenAIInfra if any GenAIExamples' docker compose files get changed(In this specific case, it's issue #484). I believe @daisy-ycguo has already enabled the CD test for GenAIInfra which we might be able to leverage for periodical CD test.


[Crash] GUI crash, if research references unregistered recipes

I have the following situation: I have a research that can change it's recipes based on what materials are available. I'd like to have a nullcheck for when it tries to render the recipes in the book, and just skip if the key isn't available instead of crashing.
I really like the way you put it together, and I think it would be a neat config option, so that I do not need to install a separate research file for each possible situation.

The problem is in my device, closing it.


Editorial: Remove repetitive notes, briefs, etc on ref attributes

Fixes #358
Changes
Leverages https://github.com/open-telemetry/build-tools/pull/204 allowing to use inherited properties of attributes when referencing them.
Merge requirement checklist

[x] CONTRIBUTING.md guidelines followed.
[x] CHANGELOG.md
[x] schema-next.yaml

Waiting for #342 to go in first to avoid a massive merge there.

@lmolkova #342 is just merged now


Entirely new index 4

OPS Build status updates of commit 01e70a2:
:clock10: Preparing: average preparing time is 39 sec(s)

OPS Build status updates of commit 01e70a2:
:clock10: Incremental building: average incremental building time is 18 sec(s)

OPS Build status updates of commit 01e70a2:
:warning: Validation status: warnings

File
Status
Preview URL
Details

enterprise-graph/index.yml
:warning:Warning

Details

enterprise-graph/index.yml

[Warning] Author Stephen Flanagan cannot be recognized in GitHub.

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.


Add Neuronx TGI

What does this PR?
This PR adds the Dockerfile and utiltilies to run TGI on AWS infernetia2. It also adds instruction on how to build the container. It includes more steps than for GPU since we need to build the optimum library first.

Discarding in favor of https://github.com/awslabs/llm-hosting-container/pull/47.


It is unclear which capacity is shown in overview page of selected node

Well right now we named all of it Output capacity:

I already know that that's not the way to go in some special cases and there's already an issue for it to clean that up. The quick and cheap way to solve this is to be consistent and also call that column Output capacity.

The quick and cheap way to solve this is to be consistent and also call that column Output capacity.

Let's do that for now. Also please round these numbers to fewer digits in the interface (not in the back-end!)

Input and output capacity are now shown as two separate fields in the technologies tab.


Receive scene metadata with uploads

Overview
This PR adds two scene metadata fields to the uploads table -- cloud cover and acquisition date.
It parses them out separately from the incoming metadata json, throws them into a sceneMetadata
attribute, and passes all of the existing metadata in unchanged.
Checklist

~Styleguide updated, if necessary~
[x] Swagger specification updated, if necessary (if I had a dollar for every time this checkbox was the only reason I updated the swagger spec...)
[x] Symlinks from new migrations present or corrected for any new migrations

Testing Instructions
This is your base test json:
{"files":["ebee-test-flight-nz233reals.tif"],
 "datasource":"c14c8e97-ba85-4677-ac9c-069cfef1f0b1",
 "fileType":"GEOTIFF",
 "uploadStatus":"UPLOADING",
 "visibility":"PRIVATE",
 "organizationId":"dfac6307-b5ef-43f7-beda-b9f208bb7726",
 "metadata":{},
 "uploadType":"LOCAL",
 "projectId": null}

Run migrations
Bring up your server
Post the test json to /api/uploads/ -- it should succeed, and the response should have nulls for sceneAcquisitionDate and sceneCloudCover in sceneMetadata
Add a bogus field to metadata ("bogus": "bogus" is fine) and post it again -- the bogus field should end up in the response's metadata
Leave bogus in there, and add "sceneCloudCover": 0.7 to metadata in the post json -- sceneCloudCover should end up in sceneMetadata, and bogus should still go into metadata
Add "sceneAcquisitionDate": "2017-04-01T00:00:00Z" to the post json -- it should succeed, and the two scene metadata fields should go to sceneMetadata, while bogus goes into metadata

Closes #2236

All based on a misreading of the original issue. Closing and starting a new branch for the actual issue. :man_facepalming: