fix: accessibility fixes for Product Detail page and dashboard

Pre-Flight checklist

[ ] Screenshots and design review for any changes that affect layout or styling

[ ] Desktop screenshots
[ ] Mobile width screenshots

[ ] Testing

[ ] Code is tested
[ ] Changes have been manually tested

What are the relevant tickets?
#171
What's this PR do?

Fixes the accessibility on the Product detail page
Fixes accessibility on the Dashboard page

How should this be manually tested?

Open the dashboard page and check the accessibility and design. There should be no accessibility errors for that specific page and there should be no notable design changes
Verify the same as above on the Product detail page.

Screenshots (if appropriate)
(Optional)

Codecov Report

Merging #176 (1839f8e) into main (05fae55) will increase coverage by 6.76%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #176      +/-   ##
==========================================
+ Coverage   90.53%   97.30%   +6.76%
==========================================
  Files         214      122      -92
  Lines        6168     2778    -3390
  Branches      297        0     -297
==========================================
- Hits         5584     2703    -2881
+ Misses        498       75     -423
+ Partials       86        0      -86

Impacted Files
Coverage Δ

static/js/containers/pages/DashboardPage.js
100.00% <ø> (ø)

static/js/containers/pages/DashboardPage_test.js
100.00% <100.00%> (ø)

main/message.py

openedx/views.py

users/admin.py

authentication/strategy.py

courses/__init__.py

openedx/api.py

fixtures/common.py

conftest.py

... and 84 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 05fae55...1839f8e. Read the comment docs.

@pdpinch, Can you please take a look at the screenshots and let me know what you think of the changes done in regard to achieving 100% Accessibility. Also, I have posted reviewer details in the PR description about the approach:

Lighthouse complains about descending order of headings to be followed for accessibility e.g. h1->h2->h3... But in our case, we were using h1 and then directly h4 so i had to make our design use the heading tag in order and for that, i had to override the style of h4 as h2 in the specific classes to keep the design intact as well as resolve accessibility issues by LightHouse. I'm aware of using the common h2 style but using h2 here as is was a notable difference
The second complaint was the color contrast so least possible color to fix the issue in tiles.

Additionally, this PR is based on targeting 100% accessibility which I'm not sure if we can compromise or not e.g.

If we don't do the tile color change for contrast we get a 98% accessibility result which is also good but is it acceptable?

Sorry, I'm not following the specifics. Which title is it that has a h4 style but needs to be an h2?
And which colors are failing the contrast test?
If it's easier, you could give me a URL of the test page and I can run lighthouse in my browser.

@pdpinch ,

Sorry, I'm not following the specifics. Which title is it that has a h4 style but needs to be an h2?

1.1 On the Product detail page the instructor name is h4

1.2 On the dashboard, the enrolled course name and Enroll Now(In course of no enrollment) are h4

And which colors are failing the contrast test?

2.1 On product detail page, The tiles background color is failing the contrast test

2.2 On the product detail page, The instructor description color is failing the contrast test (You can see the screenshot in 1.2)

If it's easier, you could give me a URL of the test page and I can run lighthouse in my browser.

For Product details failing tests mentioned above you can use https://rc.mitxonline.mit.edu/courses/course-v1:MITx+14.100x/
For dashboard failing tests, you can use https://rc.mitxonline.mit.edu/dashboard

@arslanashraf7 although, It improves the accessibility points but If I looked around our *.html templates then I figured it out we are not incorporating the ARIA semantics to most of our HTML DOM elements such as
, It would be just a suggestion if we take care of it.
But in our code base, I came to know that the IDs used in ARIA and labels are not unique as an example, under /authentication/profile.html, /authentication/login.html and authentication/register.html we defined the same ID for
 elements.

same for

I'm going to check with our accessibility expert about the heading order, but can I ask what happens if we make the instructor name an H3, and the course title on the dashboard an H2? If it's easier, you can tell me where I can see implementations of H2 and H3.

@pdpinch,

can I ask what happens if we make the instructor name an H3, and the course title on the dashboard an H2?

Their font size reasonably changes w.r.t to the design.

If it's easier, you can tell me where I can see implementations of H2 and H3.

Sure you can see the common styling of all the headings in common.scss. To be specific you can see the styling of H2 here and H3 here.
Let me know if this wasn't what you were asking.

Thanks for the details!
1.1 Please make the instructor's name into an H3
1.2. Please make the course title into an H2
I'm still getting some design guidance on the colors.

@asadiqbal08 Thanks for raising the concerns, I'm responding to them below probably you can let me know if you agree.

@arslanashraf7 although, It improves the accessibility points but If I looked around our *.html templates then I figured it out we are not incorporating the ARIA semantics to most of our HTML DOM elements such as
, It would be just a suggestion if we take care of it.

The scope of adding the ARIA IDs in all the HTML elements is very large, What is your opinion shouldn't we do that in another singular ticket, in addition, Lighthouse doesn't currently restrain about all the needing to have ARIA attr. I'd look for your response before proceeding on this.

But in our code base, I came to know that the IDs used in ARIA and DOM elements are not unique as an example, under /authentication/profile.html, /authentication/login.html and authentication/register.html we defined the same ID for
 elements.

This might not be an issue considering we only need unique Ids within a single HTML file unless some other file is getting included. So as long as we don't duplicate the Ids within a single file there should be no issue. Let me know what you think?

Your concerns regarding header should have been fixed in https://github.com/mitodl/mitxonline/pull/168.

Thanks for the details!
1.1 Please make the instructor's name into an H3
1.2. Please make the course title into an H2
I'm still getting some design guidance on the colors.

@pdpinch The mentioned changes have been incorporated and the design screenshots for your reference have been updated in the PR description as well.

InVision has been updated with higher-contrast colors: https://projects.invisionapp.com/share/KH11D3ZN53GC/#/screens/455477372/comments/145302163

2.1 On product detail page, The tiles background color is failing the contrast test

Change the background to #465A5B
2.2 On the product detail page, The instructor description color is failing the contrast test (You can see the screenshot in 1.2)
Change the text color to #666666

This might not be an issue considering we only need unique Ids within a single HTML file unless some other file is getting included. So as long as we don't duplicate the Ids within a single file there should be no issue. Let me know what you think?

@arslanashraf7, On my local setup, Lighthouse was complaining about the uniqueness of ID for DOM elements,
ARIA ids are not unique, It seems me some of the HTML DOM elements having the same ID so I mentioned it.
and just as a good practice, ID elements should be unique regardless of page rendering. I have register.html and the block under it containing a
.
If you think, it can increase the effort then I am ok to do separately.

@arslanashraf7, On my local setup, Lighthouse was complaining about the uniqueness of ID for DOM elements,
ARIA ids are not unique, It seems me some of the HTML DOM elements having the same ID so I mentioned it.

As mentioned earlier, this complaint is resolved after merging header accessibility PR

and just as a good practice, ID elements should be unique regardless of page rendering. I have register.html and the block under it containing a
.

I've updated the page Ids in register and profile to be unique.

@pdpinch The screenshots in the PR description now represent the latest changes as per this PR. Let me know if they look good to you.

InVision has been updated with higher-contrast colors: https://projects.invisionapp.com/share/KH11D3ZN53GC/#/screens/455477372/comments/145302163

2.1 On product detail page, The tiles background color is failing the contrast test

Change the background to #465A5B
2.2 On the product detail page, The instructor description color is failing the contrast test (You can see the screenshot in 1.2)
Change the text color to #666666

@pdpinch Thank you for taking it on with the designers and finalizing the changes. The screenshots in the PR description now represent the latest changes as per all the above-suggested changes for headings order & contrast. Let me know if they look good to you.

@pdpinch, Should i go ahead and merge this PR?

Yes please


Add support for api/ folder

Context & Description

This PR adds the support to the api/ folder in which the user can define any API endpoint that doesn't return any server side rendered HTML.

TODO. Add to this PR tests and write documentation about


Carry over empty entries in target cache.

The compilation seems to be failing due to the renaming of t to target.


update README to include enum34 dependency

Hmm, some CI failures related to zlib, but this is still mergeable.


Get SeqViz Working

Component Description
Import SeqViz into storybook. Set up with a basic, user controlled sequence
Time Estimate
2 hrs

@ninjha01

Closing because we're building our own sequence viewer


OIDC redirects from team site to personal page after login

Thanks a lot for OIDC! Works great in my limited tests yesterday. Public Sharing, Team site ... all works, fantastic! Only found two minor issues.
One of them...
After creating a team site...
curl -s -X POST -H "Content-Type: application/json" -d '{"name": "Outside Sensors", "domain": "outside"}' -H "Authorization: Bearer <my api key>" https://docs.grist.example.com/api/orgs

... and opening https://outside.grist.example.com, I will be redirect to Authelia (or Keycloak). After login, Authelia redirects to my personal page https://docs.grist.example.com. SAML redirects back to the team site.
This is very minor and I'm not sure if this works at all with OIDC. Only a feedback. If this can't work with OIDC, just close my issue.
Thanks.

This should be fixable. The method getLoginRedirectUrl() takes a second parameter (targetUrl: URL), which is the final destination URL. We can relay that to the end of the flow by either including it into the state parameter (along with the random value) or into the session (which seems simpler).
Let me cc @fflorent, since some of the points below are relevant to OIDC whether or not we deal with redirects.
Regarding the state parameter, it's unclear if it's actually important to security, since pkce (code_challenge / code_verifier) address the same need and more. From what I could find online though, state is still recommended to include (some of that debate is visible here: https://security.stackexchange.com/a/215027). Assuming we keep state, it seems to me that the code here isn't quite right: state should be stored in the session, to be compared inside the callback to the state passed back from the authorization server.
In other words, when saving values in the session, I think it would make sense to save this:
    mreq.session.oidc = {
      codeVerifier,
      state,
      targetUrl,
    };

Then use the state from here when calling this._client.callback(), and use targetUrl from here for the final redirect.
Something I hadn't thought of earlier, but I think should be added, is clearing mreq.session.oidc as soon as those values are used, and even in case of error. I saw this recommendation somewhere where I can't find at the moment, but it makes sense to me, since these are intended as single-use values.
It's important that attackers can't set targetUrl to anything they want -- setting it to a phishing site would be bad. I think the value of targetUrl passed in to getLoginRedirectUrl() is already safe (at least several code paths I looked at seem to take care of it); but it would be a good idea to add a test case that it's not easy to get Grist to redirect one to a random site after an OIDC login.

I've proposed a PR to fix all of this :+1:

Thanks, works 👍


Add All Payments Endpoint to CLI

Expected Behavior
stellar payments

This should also take cursor, order, and page-limit as arguments.
Payment details to display
id
to
from
asset
amount

https://www.stellar.org/developers/horizon/reference/endpoints/payments-all.html

Interested -- Self Assigning to me :)

Payments consist of many different ops. Haven't determined them all yet, but so far:

payment
create account
payment path


[WIP] Introduced Validator::format()

Refactored all validators to use $this->format instead of Yii::$app->getI18n()->format.

Q
A

Is bugfix?
yes (subjective)

New feature?
no

Breaks BC?
no

Tests pass?

Fixed issues
#13254

Will fix this tomorrow.

@dynasource I chose format since it is used by formatters as well.
Let me know if I must change it to formatMessage.

formatMessage looks better to me in this case since context isn't clear from just format.

That refactor will have to wait till tomorrow so i can do it the lazy way with an IDE :)

Merged. Thanks!


Fix halide baseline rules

lmao we just weren't parsing these right

What does this mean exactly? Are the current results wrong?

What does this mean exactly? Are the current results wrong?

This won't change the results using enumo rules to derive the halide ones.
But without the ? in the halide rules, the variables weren't being parsed as pattern variables (so for example the rule (! (! x)) ==> x would only match a variable enode literally named x). The halide->enumo derivability was saying 0 for everything (and I think we always knew this was suspect but just hadn't had a chance to dig into). This should fix that direction.

ooooooooo okay great


json-loader not needed in webpack@4

I got the following error when following the instructions from the README.md:

ERROR in ./node_modules/moment-timezone/data/packed/latest.json
Module parse failed: Unexpected token m in JSON at position 0
You may need an appropriate loader to handle this file type.

It turns out that webpack@4 automatically loads json files and hence the json-loader is not needed (I found out here https://github.com/webpack-contrib/file-loader/issues/259). The solution was to ignore the json-loader instructions from the README.md and things were working as expected. I suggest this should be mentioned in the README

@kaspermarkus
Yes, you are right. Some users may don't need to install json-loader. I'll change README ASAP. Thanks for your feedback!


Prevent App Bundles from undergoing PSP removal

For https://github.com/giantswarm/app-admission-controller/pull/354
Checklist

[ ] Update changelog in CHANGELOG.md.

cc: @giantswarm/team-atlas, this will exclude MC app bundles from being subjected to PSS mutation. WC app bundles will still get mutated.


Packaging: Drop python 3.7, update packages

Python 3.7 was dropped June 27th, 2023
Source: https://devguide.python.org/versions/

Codecov Report

Merging #13 (e5d6071) into master (e710979) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #13   +/-   ##
=======================================
  Coverage   71.79%   71.79%
=======================================
  Files           4        4
  Lines         156      156
=======================================
  Hits          112      112
  Misses         44       44

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more


Support for mk20dx128 (Teensy 3.0)?

Hi, this is perhaps an odd request but I would like to know how difficult would be to adapt Snooze for the mk20dx128 (older Teensy 3.0)?
I tried to add a TEENSY_30 src with the changed #if defined MK20DX128  and deleted the IRQ_CMP2 references but my Teensy 3.0 doesn't seem to wake up after going to sleep.
Thank you!
florin

What are trying to use to wakeup, compare? I'm not sure if the Teensy 3.0 has that though I haven't looked at Teensy 3.0 in 7 years. I can say I'm not going to support Teensy 3.0 since Paul dropped support also. Why not just buy a Teensy 3.2?

I have a couple of older boards and it's just enough for a project I'm doing.
I tried to use both the SnoozeTimer and SnoozeDigital to wake the CPU back up. There are references to a low power mode in the datasheet but not much further. Anyway, thanks for trying to help.


Users should be able to drag and drop a plant from the plant search widget into the farm designer widget

Woah. Is this old thing completed?!

WHOOOOAAAHHHH


Add quirks to support Sinope light and dimmer

First pull request for Sinope light and dimmer support in zha/zigpy

You don't need lines 38 through 53. Just use cluster id for manufacturer specific cluster feeding n signature/replacement

Oups need formating

If I remove line 38 to 53 I also need to remove line SinopeTechnologiesLightingCluster, and
 SinopeTechnologiesManufacturerCluster, in replacement.
How do I force an update of my PR to add those changes

I've closed that PR but open a new one #425. So how can I add changes to PR 425. Do I need to close and open a new one ?

Just use cluster IDs for those. You are not doing anything with the clusters themselves, you are just overriding the endpoint's device type.

Just push changes to the same branch for which you be opened PR

To fix formatting:
pip3 install pre-commit
pre-commit install
pip3 install black
black

I've install and run black to reformat then removed some unused code but I'm still stuck with
The command "travis_wait 40 tox --develop" exited with 1

About the cluster_id, I think I need to have it everywhere because I don't change any cluster. So for example I need to put
Basic.cluster_id, in both signature and replacement. If I put only Basic, I'll need to have a class extenting Basic
class Basic(Basic):
    """ Changing something in Basic cluster """
    things_to_be_changed

In your case just put Basic.cluster_id in the signature and replacement. You are not changing any clusters, are you?

You put a cluster class instead of a cluster id in the replacement, only when you want to use your CustomCkuster class because you are doing something different there, like a new command or new attribute

For now I'm not changing anything in the clusters. That's the next step
I've set the DeviceType in signature back to it's original one ON_OFF_LIGHT_SWITCH and add the new DeviceType ON_OFF_LIGHT in replacement. Then reformat with black

I've updated my PR but still getting the same error
ERROR: InvocationError for command /home/travis/build/zigpy/zha-device-handlers/script/check_format (exited with code 1)
Black formating just changed the OUTPUT_CLUSTERS: format, each cluster_id have been splitted on separate line


Failure with pillow>=10.0.0

dnstwist fails with Pillow>=10.0.0 due to backward incompatible changes. Have a look at this Debian bug report for more information on this issue.
Replacing Image.ANTIALIAS by Image.LANCZOS in this line fixes the issue for me.

No need. Constant Image.LANCZOS is present in older Pillow versions, including 7.0.0.

You are absolutely right. I should have checked it before making the suggestion. Thanks again for your speedy feedback.


⚠ Bofrost.de has degraded performance

In 33830ba, Bofrost.de (https://www.bofrost.de) experienced degraded performance:

HTTP code: 200
Response time: 1660 ms

Resolved: Bofrost.de performance has improved in 4806520 after 20 minutes.


[Refactor] AirflowTaskDependencySensor

Refactor task_dependency_sensor to support various auth mechanisms for airflow and not just oauth.
This is concerning a Databricks task waiting for airflow task to be finished executing.

completed as part of: #21


Remove USRNet from the list of supported networks

As a result of this PR: https://github.com/openvinotoolkit/openvino_contrib/pull/261

Do we really need to remove the model name from supported list?

@dkurt I don't think it's required from a security perspective. I just thought to make README aligned with tests.
If you prefer to keep it in README I think we may close this PR.

Yes, I just wanted to keep a reference to USRNet if there is no legal/security issues here.


congress-mongodb

Hello, I'd like to share this with the organization. It's inspired by the congress and congress-legislators projects as a way to combine them for use within NodeJS apps.
Looking for feedback! :)

Thanks for letting us know! This is great. Mind if we link to you on our project READMEs?

Sure, that'd be great!

On Jul 23, 2017, at 1:22 AM, Eric Mill<EMAIL_ADDRESS>wrote:
Thanks for letting us know! This is great. Mind if we link to you on our project READMEs?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.


Add Adapter Example: DynamoDB

Describe the feature
Made an example of using nuxt-auth with @auth/dynamodb-adapter from authjs.dev.
I think it would be a good additional to show some examples of others Auth.js database Adapters. For this one i used Discord as provider.
Caveat : One bug related to dynamo was the crypto library not defined in the dynamodb Adapter, i managed to replace it by the uuid package in :
<EMAIL_ADDRESS>import { v4 as uuidv4 } from 'uuid';
crypto.randomUUID() replaced by uuidv4()
Any idea of how to deal with this more properly ?
Additional information
The dynamoDB table can be generated here https://authjs.dev/reference/adapter/dynamodb#default-schema

For anyone who came here from google
I encountered in the error :
[next-auth][error][adapter_error_createUser]
https://next-auth.js.org/errors#adapter_error_createuser crypto is not defined {
message: 'crypto is not defined',
stack: 'ReferenceError: crypto is not defined\n' +
I solved the issue by setting the Node.js version to 20 in the deployment


Cannot manually specify path to Python interpreter in Command Palette

Environment data

VS Code version: 1.39.2
Extension version (available under the Extensions sidebar): 2019.10.44104 (Python)
OS and version: macOS Mojave (10.14.6)
Python version (& distribution if applicable, e.g. Anaconda): Python 3.7.4 (homebrew)
Type of virtual environment used (N/A | venv | virtualenv | conda | ...): pipenv / virtualenv

Expected behaviour
To be able to copy / paste or otherwise manually enter the path to my python interpreter (in a virtualenv) in the select interpreter prompt in the command palette.
Actual behaviour
Can only select from 3 different Python versions discovered on my system.  Current workaround is to choose a random different version, then open up settings.json and manually modify the path.
Steps to reproduce:

Create a directory outside of the working VSCode directory, and run pipenv install inside it.
Inside the pipenv directory, run pipenv run which python to show the path to the corresponding Python interpreter.
Go to the Command Palette in VSCode, and select "Python: Select Interpreter".
Attempt to copy and paste the path to the virtualenv Python interpreter (should fail).

@dwilson5, thanks for letting us know about this.  I was able to reproduce the behavior you described.  I'm going to mark this as an enhancement request.

Closing in favour of https://github.com/microsoft/vscode-python/issues/216.


Make Http2ToHttpHeaderTranslator method private.

Motivation:
Http2ToHttpHeaderTranslator is a private class but translateHeaders(Iterable<Entry<CharSequence, CharSequence>>) is public but it should be private.
Modification:
Changed public to private.
Result:
Correct access modifer.

@netty-bot test this please

@normanmaurer I think the contructor should also be private?

@netty-bot test this please


Enable-WriteCore-feature-for-netapp

Contributing to the Azure SDK
Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.
For specific information about pull request etiquette and best practices, see this section.

API change check
APIView has identified API level changes in this PR and created following API reviews.
Azure.ResourceManager.NetApp


[wrt] Update test steps for uxmanu

Impacted TCs num(approved): New 0, Update 4, Delete 0
Unit test Platform: Android
Android test result summary: Pass 4, Fail 0, Block 0
BUG=https://crosswalk-project.org/jira/browse/XWALK-4980

LGTM


Add support for --stop-containers-on-exit=false for dockerd

dockerd needs an extra param to tell it to optionally not stop containers on exit... eg:
/dockerd --stop-containers-on-exit=false
Without this, the only way to make use of this feature is to kill -9 dockerd which is very undesirable.
cc @crosbymichael
Ideally this can be included in 1.12 cause we I would like to run dockerd in our enterprise with --stop-containers-on-exit and --live-restore starting from next release.

OK I think I misunderstood / mis-tested ... it appears that "stop-containers-on-exit=false" is coupled with live restore, going to simply close this for now.


test EDSR_x2.pt error

I download pre-train model and test like this:
python main.py --data_test Demo --scale 2 --pre_train ../experiment/model/EDSR_x2.pt --test_only --save_results

but i get an error
Making model...
Loading model from ../experiment/model/EDSR_x2.pt
Traceback (most recent call last):
  File "/home/x00346096/code/pytorch/EDSR-PyTorch/code/model/edsr.py", line 64, in load_state_dict
    own_state[name].copy_(param)
RuntimeError: invalid argument 2: sizes do not match at /pytorch/torch/lib/THC/THCTensorCopy.cu:31

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "main.py", line 15, in <module>
    model = model.Model(args, checkpoint)
  File "/home/x00346096/code/pytorch/EDSR-PyTorch/code/model/__init__.py", line 40, in __init__
    cpu=args.cpu
  File "/home/x00346096/code/pytorch/EDSR-PyTorch/code/model/__init__.py", line 110, in load
    strict=False
  File "/home/x00346096/code/pytorch/EDSR-PyTorch/code/model/edsr.py", line 70, in load_state_dict
    .format(name, own_state[name].size(), param.size()))
RuntimeError: While copying the parameter named head.0.weight, whose dimensions in the model are torch.Size([64, 3, 3, 3]) and whose dimensions in the checkpoint are torch.Size([256, 3, 3, 3]).

Hello.
You have to put additional arguments(--n_resblocks, --n_feats, and --res_scale) to test that model.
Please refer to this line!
Thank you.

I recommend you to use --chop argument if there is any trouble with GPU memory.

I get the same error when trying to test on the EDSR_x4.pt model
Regarding your answer:

You have to put additional arguments(--n_resblocks, --n_feats, and --res_scale) to test that model.
What do these parameters need to be set to?

Additional description
This happens because PyTorch does not save model structure when we make checkpoints.
It means that pre-trained models only contain parameters, but not structures.
Therefore, we have to build the model first to load those parameters.
Default setting is EDSR_baseline_x4, which has 16 resblocks and 64 channels per layer.
If you do not specify those arguments, an error occurs because you are trying to load the parameters from the model with 32 resblocks and 256 channels per layer.

The problem has been solved.
Thank you.


build_docker_node failing in interop tests

https://source.cloud.google.com/results/invocations/8f98c203-a9ee-43b1-aa85-cf3f375e6cb8/targets
https://source.cloud.google.com/results/invocations/5d5d5adf-fc93-4e7c-8f95-0a3a3634ebab/targets

Similar to #15289, but the error seems to be  fatal: reference is not a tree: 2068ffe512795d16dc31522c5531075a4f239e7d
Fetching submodule third_party/benchmark
Fetching submodule third_party/boringssl-with-bazel
Fetching submodule third_party/protobuf
fatal: reference is not a tree: 2068ffe512795d16dc31522c5531075a4f239e7d
+ EXITCODE=128
+ docker rm -f build_grpc_interop_node_7595ade7-06fd-4b1c-8a19-7ddf31bdcbe7
build_grpc_interop_node_7595ade7-06fd-4b1c-8a19-7ddf31bdcbe7
+ exit 128
2018-04-25 13:29:43,639 FAILED: build_docker_node [ret=128, pid=9361, time=312.0sec]

After inspecting logs for some of the PRs referencing this issue, I think the fatal: reference is not a tree: 2068ffe512795d16dc31522c5531075a4f239e7d might be a one-off, and most of the failures are actually
instances of #15289 with error No rule to make target 'Release/obj.target/grpc/deps/grpc/src/core/ext/filters/client_channel/lb_policy/subchannel_list.o'

Hmm.  I did remove subchannel_list.cc in #14886, but I did update the build files, and it looks like this test was failing even before that.  So I'm not sure if that's related.

I'll close this issue at #15289 seems to be the one that's currently ongoing.


cuda memory insufficient

Hi, thanks for your work!
I am trying test.py on gpu, but got error "cuda out of memory".(rtx 2080ti)
Any solution could solve this?
thanks!

As far as I know and with my limited hardware (GTX 1080), I am not able to solve this in any way.
However, you may take a look at the discussion in the PyTorch forum.
https://discuss.pytorch.org/t/solved-pytorch-conv2d-consumes-more-gpu-memory-than-tensorflow/28998
I am closing this since it is more of a PyTorch issue.


Event bug

The Eris contracts library (or something deeper) does not appear to be handling solidity events properly. The branch #29 has an integration test for showing how the event subscription does not appear to be being called.

This looks like it's being caused by https://github.com/eris-ltd/eris-db/issues/96.


feat(balancer) adds consistent hashing description

related to PR https://github.com/Kong/kong/pull/2875

@thibaultcha comments addressed.


support for optional timeout argument in all TM1py functions

All methods in TM1py should support an optional argument: timeout:
data = tm1.cubes.cells.execute_mdx(mdx=mdx, timeout=120)
If the TM1 Server does not respond in a timely manner, TM1py will throw a TimeoutException.
This allows to circumvent hanging situations on the client side, when the request may be locked or working for a long time in the Server.

+1
Here is the patch file.
Timeout was added with the TM1Service logon and so is the same for the length of the session (until logout).
tm1py_timeout.zip
I've tested it with GET, POST, DELETE and it works with or without timeout argument.

Hi @csjean,
It is great that you have provided a patch, welcome as a contributor!
The appropriate way to contribute is to fork TM1Py and then create a pull request with your changes. That makes it transparent who has made changes and also reduces the security risk of zip files of unknown origin.
Instructions are below:

https://help.github.com/en/articles/fork-a-repo
https://help.github.com/en/articles/creating-a-pull-request-from-a-fork

Thanks @cubewise-tryan .
I have created a pull request.
Cheers!

@csjean, excellent! We really appreciate it.

solved with #115


Update version in manifest to 0.4.7 for release

I've just tested everything and it looks good! It should be released in a few hours


使用 playground app 跑 list 的例子，安卓手机头部样式不正确

hello,
我运行官网的 list 的例子，发现固定的 header 头部样式在手机上不正常，具体见截图：

我也碰到，而且不仅是样式，有时候根本无法固定顶部

遇到同样的问题，本来布局好好的，playground的头会占用屏幕

This PR/issue doesn't receive response since Weex migrated to Apache two years ago and this repository is no longer active yet.
Feel free to ask question in new repository and thanks for your contribution.

This issue is outdated for a long time and will be closed now. You can create a new one if you still have questions.

This issue is outdated for a long time and will be closed now. You can create a new one if you still have questions.


Scanning after updating the Trivy database

Hi, please tell me, if the Trivy database has been updated, will all deployments be scanned again?

Nope. Currently such mechanism is not implemented. One way to rescan workloads is to delete VulnerabilityReport instances, which will trigger rescan.


Comma as a separation character messes with some localized float representation.

On some localized systems (e.g. mine) floats have comma(,) separating integer part from the fraction part instead of dot(.). Because of that separating parameters in GetSettingsString/SetSettingsString in SfxrParams.cs with comma(,) creates a string that isn't read correctly. Please, change that symbol to something else.
I hope, my explanation was adequate, I have limited programming knowledge and English isn't my first language.
Thank you.

Sounds like something needs to be using InvariantCulture that isn't. If that is a string being manually editted by the user they need to be entered as InvariantCulture.


Please consider fixing issues detected by repository checker

Notification from ioBroker Check and Service Bot
Dear adapter developer,
I'm the ioBroker Check and Service Bot. I'm an automated tool processing routine tasks for the ioBroker infrastructure. I have recently checked the repository for your adapter fuelpricemonitor for common errors and appropiate suggestions to keep this adapter up to date.
This check is based the current head revisions (master / main  branch) of the adapter repository
Please see the result of the check below.
ioBroker.fuelpricemonitor
   -
ERRORS:

[ ] :heavy_exclamation_mark: [E519] "@alcalzone/release-script" requires plugin "@alcalzone/release-script-plugin-license". Please add.

WARNINGS:

[ ] :eyes: [W515] Why did you decide to disable i18n support?

SUGGESTIONS:

[ ] :pushpin: [S519] Consider adding plugin "@alcalzone/release-script-plugin-manual-review".

Please review issues reported and consider fixing them as soon as appropiate.
Errors reported by repository checker should be fixed as soon as possible. Some of them require a new release to be considered as fixed. Please note that errors reported by checker might be considered as blocking point for future updates at stable repository.
Warnings reported by repository checker should be reviewed. While some warnings can be ignored due to good reasons or a dedicated decision of the developer, most warnings should be fixed as soon as appropiate.
Suggestions reported by repository checker should be reviewed. Suggestions can be ignored due to a decision of the developer but they are reported as a hint to use a configuration which might get required in future or at least is used be most adapters. Suggestions are always optional to follow.
You may start a new check at any time by adding the following comment to this issue:
@iobroker-bot recheck
Please note that I (and the server at GitHub) have always plenty of work to do. So it may last up to 30 minutes until you see a reaction. I will drop a comment here as soon as I start processing.
Feel free to contact me (@iobroker-bot) if you have any questions or feel that an issue is incorrectly flagged.
And THANKS A LOT for maintaining this adapter from me and all users.
Let's work together for the best user experience.
your
ioBroker Check and Service Bot
Note: This issue replaces issue #707
@mcm1957 for evidence

@iobroker-bot recheck

Thanks for contacting me.
I received your request to recheck this repository. I will start doing my work soon.
I will close an existing issue and create a new one if still any issues are to be evaluated and potentially to be fixed.
Thanks for spending your time and working at an ioBroker adapter.
your
ioBroker Check and Service Bot

This issue has been replaced by new isse #714
This issue can be closed.
your
ioBroker Check and Service Bot


add support for preparedStatement.addBatch()

Hi, Any progress on that one?

Hi Michael,
Just want to check update/progress on this? As I mentioned to Neo4j support team, this driver helps us a lot. Combined with Spring framwork,

The error handling is much simpler.
Database transactions are enabled with Spring framework's transaction manager, it's free.
Using JDBC in conjunction with Spring framework's JdbcTemplate is convenient and less coding. With Spring's RowMapper, it's almost free.
Different database flavors, unit testing with embedded, integration testing with file backed, production with standalone, without alternating production code.

The only feature we found missing in our ecosystem is the batch support since we do have a scenario in which we need to insert several hundreds of node/relation in one shot.
So we want to see whether there is any chance to bump the priority of this ticket.
Thanks a lot.
Rick


189907836 General Comment (Clare Pierson)

Issue reported by Clare Pierson at 2021-01-07 00:10:36 UTC
Time: 2021-01-07T00:06:20+00:00
Session ID:
Problem Page URL:
Previous Page URL: https://www.freecen.org.uk/contacts/new
Reported Issue:
How on earth do I get no results when I search for McQuilkin in County Antrim census records??  Several McQuilkins lived in County Antrim, some in East Torr and Culfeightrin Civil Parish. I already have a copy of one census, but not of the McQuilkin for whom I want more information.

Contact enquirer that we're temporarily removing Ireland from the search.

Contact enquirer that we're temporarily removing Ireland from the search.

Replied to Clare, closing

Replied to Clare, closing


Implement en-GB Locale

This is missing from the backlog identified in #11 and can be directly added, as d3-format has bindings for it.

This has been added in build 122, so will be supported from 0.5 onwards:


Add DefaultReadmeGenerator

Description of changes:
(See previous closed PR https://github.com/awslabs/smithy-typescript/pull/584, opening a new PR from fork repo)

Added DefaultReadmeGenerator
Added createDefaultReadme in TypeScriptSettings (defaults to false)
This generates a default README.md if createDefaultReadme is set  to true in smithy-build.json.
Validated README.md is created in both SDK and SSDK (via smithy-typescript-ssdk-demo)

See client and server README examples below:

weather
Description
SDK for JavaScript Weather Client for Node.js, Browser and React Native.
Provides weather forecasts.
Installing
To install the this package, simply type add or install weather
using your favorite package manager:

npm install weather
yarn add weather
pnpm add weather

Getting Started
Import
To send a request, you only need to import the WeatherClient and
the commands you need, for example GetCityCommand:
// CJS example
const { WeatherClient, GetCityCommand } = require("weather");

// ES6+ example
import { WeatherClient, GetCityCommand } from "weather";

Usage
To send a request, you:

Initiate client with configuration.
Initiate command with input parameters.
Call send operation on client with command object as input.
If you are using a custom http handler, you may call destroy() to close open connections.

// a client can be shared by different commands.
const client = new WeatherClient();

const params = { /** input parameters */ };
const command = new GetCityCommand(params);

Async/await
We recommend using await
operator to wait for the promise returned by send operation as follows:
// async/await.
try {
  const data = await client.send(command);
  // process data.
} catch (error) {
  // error handling.
} finally {
  // finally.
}

Async-await is clean, concise, intuitive, easy to debug and has better error handling
as compared to using Promise chains or callbacks.
Promises
You can also use Promise chaining
to execute send operation.
client.send(command).then(
  (data) => {
    // process data.
  },
  (error) => {
    // error handling.
  }
);

Promises can also be called using .catch() and .finally() as follows:
client
  .send(command)
  .then((data) => {
    // process data.
  })
  .catch((error) => {
    // error handling.
  })
  .finally(() => {
    // finally.
  });

Callbacks
We do not recommend using callbacks because of callback hell,
but they are supported by the send operation.
// callbacks.
client.send(command, (err, data) => {
  // process err and data.
});

Troubleshooting
When the service returns an exception, the error will include the exception information,
as well as response metadata (e.g. request id).
try {
  const data = await client.send(command);
  // process data.
} catch (error) {
  const { requestId, httpStatusCode } = error.$$metadata;
  console.log({ requestId, httpStatusCode });
  /**
   * The keys within exceptions are also parsed.
   * You can access them by specifying exception names:
   * if (error.name === 'SomeServiceException') {
   *     const value = error.specialKeyInException;
   * }
   */
}

@bootleg-service/server-bootleg
Description
JavaScript Server SDK for Bootleg
Installing
To install this package, simply type add or install @bootleg-service/server-bootleg
using your favorite package manager:

npm install @bootleg-service/server-bootleg
yarn add @bootleg-service/server-bootleg
pnpm add @bootleg-service/server-bootleg

Getting Started
Below is an example service handler created for the CreateVenue operation.
import { createServer, IncomingMessage, ServerResponse } from "http";
import { HttpRequest } from "@aws-sdk/protocol-http";
import {
  BootlegService as __BootlegService,
  CreateVenueInput,
  CreateVenueOutput,
  getBootlegServiceHandler
} from "@bootleg-service/server-bootleg";
import { convertEvent, convertResponse } from "@aws-smithy/server-node";

class BootlegService implements __BootlegService {
  CreateVenue(input: CreateVenueInput, request: HttpRequest): CreateVenueOutput {
    // Populate your business logic
  }
}

const serviceHandler = getBootlegServiceHandler(new BootlegService());

const server = createServer(async function (
  req: IncomingMessage,
  res: ServerResponse<IncomingMessage> & { req: IncomingMessage }
) {
  // Convert NodeJS's http request to an HttpRequest.
  const httpRequest = convertRequest(req);

  // Call the service handler, which will route the request to the GreetingService
  // implementation and then serialize the response to an HttpResponse.
  const httpResponse = await serviceHandler.handle(httpRequest);

  // Write the HttpResponse to NodeJS http's response expected format.
  return writeResponse(httpResponse, res);
});

server.listen(3000);

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Gosar @AndrewFossAWS
Hi everyone,
Firstly, thank you for this library; it has been incredibly useful. I'm setting up my Smithy models and noticed that the option createDefaultReadme=false seems to only work for the server generator. I tried disabling the README generation because the default content includes information primarily relevant to AWS SDK clients, which doesn't align with my use case.
Thanks again for your work on this library!
This is my config:
{
    "version": "1.0",
    "outputDirectory": "build/",
    "sources": ["models"],
    "maven": {
        "dependencies": [
            "software.amazon.smithy:smithy-model:1.50.0",
            "software.amazon.smithy:smithy-linters:1.50.0",
            "software.amazon.smithy:smithy-openapi:1.50.0",
            "software.amazon.smithy:smithy-openapi-traits:1.50.0",
            "software.amazon.smithy:smithy-aws-apigateway-openapi:1.50.0",
            "software.amazon.smithy:smithy-aws-traits:1.50.0",
            "software.amazon.smithy:smithy-aws-cloudformation:1.50.0",
            "software.amazon.smithy.typescript:smithy-typescript-codegen:0.21.1",
            "software.amazon.smithy.typescript:smithy-aws-typescript-codegen:0.21.1"
        ],
        "repositories": []
    },
    "projections": {
        "configuration": {
            "plugins": {
                "openapi": {
                    "service": "com.mycorp.api#Configuration",
                    "apiGatewayDefaults": "2023-08-11",
                    "protocol": "aws.protocols#restJson1",
                    "apiGatewayType" : "REST",
                    "disableCloudFormationSubstitution": true
                },
                "typescript-client-codegen": {
                    "package": "@mycorp/smithy-configuration-api-client",
                    "packageVersion": "0.0.4",
                    "packageManager": "npm",
                    "createDefaultReadme": false,
                    "generateTypeDoc": true,
                    "service": "com.mycorp.api#Configuration"
                },
                "typescript-server-codegen": {
                    "package": "@mycorp/smithy-configuration-api-server",
                    "packageVersion": "0.0.4",
                    "packageManager": "npm",
                    "disableDefaultValidation": true,
                    "createDefaultReadme": true,
                    "generateTypeDoc": true,
                    "service": "com.mycorp.api#Configuration",
                    "packageJson": {
                        "files": [
                            "dist-*/**",
                            "openapi.json"
                        ]
                    }
                }
            }
        }
    }
}


Improve TypeScript typings for withTheme HOC

This PR slightly improves typings for TypeScript of withTheme HOC.
The issue with previous definitions was in the fact that TypeScript did not allow removing required property from a generic type. However, that is exactly what withTheme does with a component's contract. It takes a component that needs theme prop (encoded as a required prop in the component) and makes another component that simply gets the theme from the context, so basically it removes it from the contract. Following the implementation logic, theme is not really removed, it's being made optional (because a consumer can override the theme by passing it directly as a prop).
To work this constraint around, previously we used theme defined as an optional, which is obviously not what we wanted. Now, you can see this from test changes, it can be declared as required.
The change is based on great type operator helpers by @Pinpickle that opened up this possibility.
Here's details on changes:

typescript upgraded to 2.4.1; the helpers do not work on previous versions, but they are not introducing regression
made tests of withTheme more strict, particularly made theme a required prop
added overload of styled function to also act like withTheme; because, in fact, an inner component can rely on theme prop defined in the context.
changed withTheme definition (themed and non-themed variations)
removed T generic parameter from non-themed variation of withTheme, which was never working and looks like a type

/ping @styled-components/typers

nobody reviews :(

I'm busy with EuroPython! I'll check when I get back, next week :)
On Thu, Jul 13, 2017, 11:04 PM Igor Oleinikov<EMAIL_ADDRESS>wrote:

nobody reviews :(
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
https://github.com/styled-components/styled-components/pull/982#issuecomment-315201895,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAotlTzhfwguUXqn7WcDeGOhRrpGMr-hks5sNoZcgaJpZM4OTmvG
.

--
Patrick

sounds good @patrick91


MLOps Initiative: Design KFServing + iter8 integration

Flesh out thin iter++ controller based demo scenarios with KNative services (created from KFServing InferenceService CRs)

Goal = eliminate need for KFServing router extensions and replace it with iter8
Emphasize multiple versions / different types of experiments / and MAB algorithm

Start design doc


Patch 1

Change Summary
The mentioned codes have been added with the documentations
Checklist
If you haven't fulfilled the below requirements or even delete the entire checklist, your PR won't be reviewed and will be closed without notice.
General

[x] This Pull Request is all my own work. (You'll be blacklisted if you are caught for plagiarism.)
[x] I've read CONTRIBUTING.md
[x] I've made some valid changes to the CheatSheet, and they are not just minor changes.

Changes

[x] I've read and followed the Solution Template

Those are some really good changes! 🍁


LDAP user with cyclic group membership causes infinite loop and GC thrashing

We have some users in Active Directory with circular group memberships:
Group A
  |
  +-- Group B
         |
         +-- Group C
                |
                +-- Group A (cycle)

When such a user authenticates, the process appears to enter an infinite loop.  I'm not certain the loop is actually infinite, but it does begin pulling gigabytes from LDAP over several minutes, enters GC hell, and stops responding to SIGTERM.  SIGKILL is our corrective measure (which is obviously problematic for other reasons). Packet captures indicate continuous traversal of groups involved in the cycle.
Interestingly, setting resolve_nested_roles to false doesn't mitigate.  (Although we need that functionality, I tried disabling it as an experiment / act of desperation.)
I suppose there needs to be some sort of "seen list" to prevent traversal into already-visited groups.

Please provide stack trace (jstack  > stact.txt)

Hi @vrozov, thanks for taking a look at this.
I believe I have oversimplified the conditions to recreate the problem.  I've tried to reproduce things using my own account by creating the circular membership described above, but no dice.
I managed to flag down the affected user today to get him to reauthenticate and grabbed two stack traces:
jstack-before.log: a baseline taken before the user authenticated
jstack-during.log: taken a minute or so after the user attempted authentication, after about 500MB of data was exchanged with LDAP.
I've also included an excerpt from the packet capture (just a copy/paste from Wireshark's summary view), with sensitive details redacted, to give you a sense of what's happening, if you have the patience to look at it. :)
wireshark.txt
The user in question is a direct member of AAAA SW Stakeholders which itself is a member of the AAAA Stakeholders group, and this is where the endless loop seems to begin.  AAAA SW Stakeholders is not revisited, but AAAA Stakeholders is revisited after a protracted journey through several other nested groups, and the adventure repeats ad nauseum until the process is killed.  (Possibly the loop would terminate after 15 minutes, but I've not waited longer than that as service is impaired during this time.)
Another idea -- and possibly a good idea in any case -- would be to provide a configurable recursion depth.  This would be an easy way to address this problem, and might also be useful for other reasons.  For example, our users complain about very slow login times (takes about 20-30 seconds), with typical accounts pulling about 20-40 MB from LDAP after many round trips.  (Our organization makes extensive use of AD groups and has plenty of real-world data messiness, as I'm sure you noticed.)  Although we do need to support nested roles for our ODFE cluster, in practice we only need to resolve to a depth of about 5.
So a configurable recursion depth limit could fix this problem as well as potentially improving our login times.
Thanks!

Hi @jtackaberry, there is a configuration that limits nested depth for ldap role resolution. Please try setting max_nested_depth in the LDAP authorization config. The default value is 30.

Ah!  The affected user is offline at the moment but once I can get him to test again I'll follow up.
However, I think I have some reason to be pessimistic: looking at jstack-during.log I notice there are exactly 30 nested occurrences of:
    at com.amazon.dlic.auth.ldap.backend.LDAPAuthorizationBackend.resolveNestedRoles(LDAPAuthorizationBackend.java:1020)

So that tells me we are hitting the default limit of 30, and despite that we're still toppling the node.
This suggests any limit may behave the same way.  I'll report back once I have the user test it, but perhaps tracking previously visited roles and preventing recursion if the role has been visited before is the most robust solution after all.
Thanks much @vrozov

Setting max_nested_depth to 5 mitigates this issue and the user can now authenticate without killing the node.  It still pulls an atypically large amount of data from LDAP (about 50% more than average) but at least the authentication completes and the node stays healthy. 👍
I do still think tracking a visited list to prevent recursion into already-visited roles is ideal though.
Thanks!

I do still think tracking a visited list to prevent recursion into already-visited roles is ideal though.

I agree. Please keep this issue open till the full fix is implemented.

We encountered the same issue. Have any plan to fix this?
Thanks,

We are doing some "spring cleaning in the fall", and to make sure we focus our energies on the right issues and we get a better picture of the state of the repo, we are closing all issues that we are carrying over from the ODFE era (ODFE is no longer supported/maintained, see post here).
If you believe this issue should still be considered for current versions of OpenSearch, apologies! Please let us know by re-opening it.
Thanks!


unmet accuracy

Thanks for this implementation!
I ran the test code with
python main.py --exp_name=test --num_points=1024 --use_sgd=True --eval=True --model_path=checkpoints/best/models/model.t7 --test_batch_size=4

(changed test_batch_size to 4 from 8 due to GPU memory constraints)
But the accuracy is really bad:
Test :: test acc: 0.483387, test avg acc: 0.455517

The only thing i did slightly differently to the original code is that, I downloaded the h5 data manually instead of using the code, as otherwise i met certificate issues.
Can anyone else replicate the 93% accuracy as reported in the repo?
update
setting --test_batch_size=1 achieves test acc: 0.932739, test avg acc: 0.899901
another observation, if test_batch_size > 1, prediction result is not reproducible / deterministic.
I'm using pytorch 1.6.0

Thanks for your point,
I find some bugs when batch_size=1, but I can't replicate


[REQUEST] sync FusedAdam with the upstream

https://github.com/microsoft/DeepSpeed/blob/43d58d99eba800a809f16efa30a90eb25f7a7003/deepspeed/ops/adam/fused_adam.py#L4-L5
needs to be synced with https://github.com/NVIDIA/apex/blob/master/apex/optimizers/fused_adam.py as since it was forked several fixes were made and BF16 support was added https://github.com/NVIDIA/apex/commits/master/apex/optimizers/fused_adam.py
the fork happened at: https://github.com/NVIDIA/apex/commit/a109f85 Sep 28, 2020
@jeffra

A small update on this thread @stas00, @awan-10 just discovered an issue with the latest version of FusedAdam from apex that has to do with this PR: https://github.com/NVIDIA/apex/pull/1560.
Once we have that resolved though i think the path towards updating this will become easier. We'll keep you posted and update this thread.

Thank you, Jeff
And FYI pytorch-2.0 now has a built-in fused version as well! we have just integrated it into transformers:
https://github.com/huggingface/transformers/pull/22144


[Bug]: Double Scrollbar when viewing KaTex inside markdown

Describe the bug
When I try to view a markdown file that have KaTex in it I got 2 scrollbars,
I already installed KaTex Preview, it does work when the Math text is not that long
but after more lines added I got the second scrollbar appearing there
probably to scroll the KaTex preview then the original scrollbar is just there not moving at all
To Reproduce
Steps to reproduce the behavior:

Go to a markdown file
Preview it
Got 2 scrollbars

Expected behavior
Normally it should only have one scrollbar
Screenshots

Environmental information
OS: Windows 11 21H2
Additional context
Just a simple scrollbar error

OK, thank you very much for your submission. This issue will be fixed in the next version.


Agent: each hostedcluster provider should have it's own role binding

What this PR does / why we need it:
Create rolebinding in agent namespace for each hostedcluster that wants to use this namespace
Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #
Checklist

[x] Subject and description added to both, commit and PR.
[ ] Relevant issues have been referenced.
[ ] This change includes docs.
[ ] This change includes unit tests.

/lgtm

/approve

changed by https://github.com/openshift/hypershift/pull/1137


metaData identifiers

For the pipeline import/export to work, I need a way to track a file between exports and imports. Currently this was done using the file name, but that means the user cannot rename the file. I found a more robust solution using DICOM identifiers. The SOPInstanceUID was perfect because it is unique per file but unfortunately I found that medInria does not actually store this metaData in its database. The second solution was to use the series DICOM ID, but while medInria does store this one, the readers and writers do not read/write it.
These identifiers should not be modified by medInria, so importing a file and then exporting it should maintain their values.

The PRs https://github.com/LoicCadour/medInria-public/pull/95 and https://github.com/LoicCadour/medInria-public/pull/102 only copied a subset of the metaData, but I still don't quite understand why it wasn't possible to simply copy all the ones contained in the data.

I just tried adding the reading/writing of all the metaData for ITK images and it works perfectly fine.

Can you do a PR if it's working? It could be interesting indeed :)

Already working on it

The VTK file format limits the header size to 256 characters, which only allows a few metadata to be stored, so even the current version doesn't save them all (when testing this sort of stuff, simply saving and loading files isn't enough, you have to manually print every metadata to make sure there's nothing wrong)

Solved by https://github.com/Inria-Asclepios/medInria-public/pull/51 and https://github.com/Inria-Asclepios/medInria-public/pull/48


Fix misc minor

Description

Just remove some useless pragmas and comments

Codecov Report

Merging #153 (a51a7c8) into master (67a9596) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #153   +/-   ##
=======================================
  Coverage   73.17%   73.17%
=======================================
  Files          86       86
  Lines        4392     4392
=======================================
  Hits         3214     3214
  Misses       1178     1178

Impacted Files
Coverage Δ

include/tweedledum/Target/Device.h
46.56% <ø> (ø)

src/Synthesis/xag/xag_synth.cpp
94.60% <ø> (ø)

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 67a9596...a51a7c8. Read the comment docs.


Add .idea to gitignore

Just a small one... I have included the .idea folder in the .gitignore for developers who use any editor from the IDEA family like myself. I figured if something like .DS_Store is in there then it's probably useful to have this 😄.

Sounds good - fwiw you can add .idea to your global .gitignore as well 👍


feat: use retry-once-with-cleanup=true for tf validate

Description
terraform_validate pre-commit fails in a case that examples has old terraform modules. To prevent this, add retry-once-with-cleanup=true flag for tf_validate which will remove problematic .terraform folders
https://github.com/antonbabenko/pre-commit-terraform#terraform_validate
Release required?

[ ] No release
[ ] Patch release (x.x.X)
[ ] Minor release (x.X.x)
[ ] Major release (X.x.x)

Release notes content

Run the pipeline
If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.
Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:
/run pipeline

Checklist for reviewers

[ ] If relevant, a test for the change is included or updated with this PR.
[ ] If relevant, documentation for the change is included or updated with this PR.

For mergers

Use a conventional commit message to set the release level. Follow the guidelines.
Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
Use the Squash and merge option.

/run pipeline


Cancel Button and OnKeyboardClicked Events

Even if the buttons are visible and clickable there are no events for the CancelButton and the Keyboard.

What control are we talking about here again??

Sorry i that i didn't mention it. The FreakyPinCodeControl. I saw that in the wiki describes an Event called Keyboard_Clicked, tried to find it in previous versions but doesn't exist.

It will be great to include those two events!

tried to find it in previous versions but doesn't exist.

What version are you on currently?

tried to find it in previous versions but doesn't exist.

What version are you on currently?

0.4.10

I just checked the documentation and I have not mentioned any of these events, is this a feature request?
https://github.com/FreakyAli/Maui.FreakyControls/wiki/FreakyPinCodeControl

I just checked the documentation and I have not mentioned any of these events, is this a feature request?
https://github.com/FreakyAli/Maui.FreakyControls/wiki/FreakyPinCodeControl

It is but also if you look at the remarks you mentioned Use event handlers like Keyboard_Clicked to update the code value when virtual keyboard buttons are pressed.. Unless you are not talking about that then it is my fault.

That is actually an internal event that I forgot to mention, Anyway I can do this whenever I make the next release, also please in case of future bugs follow the bug template thanks!

@DgkanatsiosMasoutis FYI your changes are in the PR, the next pre-release will be released this weekend 😄
https://github.com/FreakyAli/Maui.FreakyControls/blob/377292e958b4fd977f1352db21df797dbdac9618/MAUI.FreakyControls/MAUI.FreakyControls/Shared/FreakyCodeView/FreakyPinCodeControl.xaml.cs#L10

No there are 3 events, for the numbers, the cancel button and the backspace button, hope this helps

No there are 3 events, for the numbers, the cancel button and the backspace button, hope this helps

It helps a lot! Many thanks!

@DgkanatsiosMasoutis Checkout the latest release


define a wrenStringToNum() for foreign methods?

Maybe it's a good idea to put the logic from DEF_PRIMITIVE(num_fromString) into a new wren... function, so this functionality is available (like wrenNumToString) to foreign methods

Can you show a use case? I don't see a real one where it is not better to force the user to use Num::fromString() from wren language and deal with exceptions/errors from there.

A few reasons:

There is wrenNumToString, so it seems intuitive to have wrenStringToNum too.
Num::toString() uses wrenNumToString anyway, so why not have Num::fromString() use wrenStringToNum (or to use the same naming, maybe wrenNumFromString, but thats another discussion)
Converting strings to numbers is a very common use case. For example getting input from stdin as a number, parsing a serial number like 'ABC-123' and split it in a string and number, and so on...

This way, this functionality would be 'globally' available through a wren... function instead of having to use strtod or any other string-to-num function.

While I agree with all these arguments, they only are profs of form and not presence.
There are quite a number of use-case of strtod in the code base, in particular in the compiler, that would tend to validate its existence.
In the other hand there is only one caller of wrenNumToString so maybe it should be inlined.

I don't see a real one where it is not better to force the user to use Num::fromString() from wren language and deal with exceptions/errors from there.

I think I generally agree with @mhermier here. In most cases, it should be cleaner to do the conversion in the Wren side of the code. If it's a pain, though, I could see adding support for this to the API.
I should point out that wrenNumToString() is not currently part of the public API for Wren. If it was, it would be declared in "wren.h". Its name starts with "wren" because it's used by other source files in the VM. That means it needs to be exported and visible to the linker and I want to ensure it doesn't collide with other functions in the host application. The only functions in Wren that aren't prefixed are static functions that are local to a single compilation unit.


Feature/model reference

Codecov Report

Merging #40 into master will decrease coverage by 9.39%.
The diff coverage is 16.56%.

@@            Coverage Diff            @@
##           master      #40     +/-   ##
=========================================
- Coverage   51.78%   42.38%   -9.4%
=========================================
  Files          14       18      +4
  Lines         448      611    +163
  Branches       82      109     +27
=========================================
+ Hits          232      259     +27
- Misses        198      332    +134
- Partials       18       20      +2

Impacted Files
Coverage Δ

src/services/vscodeService.ts
0% <0%> (ø)

src/services/modelReferenceService.ts
0% <0%> (ø)

src/language/modelReference.ts
0% <0%> (ø)

src/common/ast/index.ts
75% <100%> (+1.66%)
:arrow_up:

src/common/ast/modelReference.ts
86.66% <86.66%> (ø)

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 8381ba7...86e6c4f. Read the comment docs.


Reverb disappears upon dying

when I die, The reverb doesn't work. It just disappears. There's no lua error or anything.
Im using the CW2.0 weapons, but it also affects the hl2 weapons.

What branch?

what?

Ok I assume you're using the v1 branch which is abandoned. Very soon we're going to release the newest version and hopefully your issue wont persist.


Add missing input to task "test"

Hi
This pull request fixes the incremental task test.
Beyond test code, this task depends on some test resources included in the src/test/fixture directory. However, this directory is not declared as input to this task, so tests  are not executed whenever there are updates to any of those dependent  files.
This commit fixes this issue by declaring src/test/fixture as input of task test.

Coverage remained the same at 95.335% when pulling 95e5c903775635110efedc92478dca102fdaf012 on theosotr:fix into aa590aa0a5a76665d012d3a386afc14741bc2937 on kt3k:master.


Fixing Firefox Download Issue (issue #37)

In order for the click event to work in Firefox as intended on an element (e.g. prompt for file download), it has to reside on document.body.  Also tested XENA on Chrome after altering codebase -- works as expected.
Represented in issue https://github.com/acthp/ucsc-xena-client/issues/21

I'm guessing this isn't an ideal solution, but I also am not seeing (nor am I aware of) any side-effects of adding and subsequently removing the newly-created element.


import kubevirt.io/api/core/v1 once

Too often, we import the same module twice, in two different names. This adds to the mental burden of the code reader.
NONE

/approve


Make a single all-in-one Debian package

A single Debian package would make it easier to download and install ungoogled Chromium; so that one package would hold the main stuff and another would contain the debugging symbols.

I don't think ungoogled-chromium-debian should deviate too far from Debian's Chromium packaging. If we want to implement this, I think this should be implemented in ungoogled-chromium-portablelinux. That way, the resulting .deb package will be usable by any Debian-based distro. I believe there should be some tool that would make this easy to integrate with our existing build scripts?

@Eloston I don't see any Debian packaging config here? so how are the debs built?
fethcing the whole Chromium source?

@oss-assist

I don't see any Debian packaging config here?

That's correct, there isn't any packaging config. I am proposing that we add packaging config to ungoogled-chromium-portablelinux.


Gfx upscaling

A few items added:

Customizable Target Resolution
Resizable Window Rendering
Upscaling Modes (Preserve / Stretch)

For reference, related issues:

https://github.com/pumpitupdev/pumptools/issues/9
https://github.com/pumpitupdev/pumptools/issues/16

FYI, you need to rebase once your other PR #54 is merged. The first commit mirrors the contribution in that other PR.


More updates to make Python 3 compatible

Added Wancho, Indic Siyaq Numbers, and Mayan Numerals to noto lint

Also added update from https://github.com/googlefonts/nototools/pull/484 so that pull request can be closed.

Thank you!


Delete messages

You can create new Messages with the modeler but I haven't found a possibility to delete messages that are not longer needed with in the graphical view.
Deleting lines like the following works, but does not guarantee that the Message is not used anywhere else in the model:
<bpmn:message id="Message_1pj78vk" name="Message_11cheio" />
I would like to have an easy and save way to clean up models from unused messages.

@falko: Note from the developer training

I believe this is a duplicate of https://github.com/camunda/camunda-modeler/issues/136, could you confirm it?

confirmed


Display multiblock in Jade (or Waila) instead of machine core

Describe the feature you'd like
With all of the multiblock machines in Oritech, it would be nice if Jade could display any of the blocks in the machine as if it was the machine instead of displaying the original "machine block" when looking at part of the multiblock machine.

Describe alternatives you've considered
No response
Additional context
No response

#82 (included in v0.9.0) shows both the machine core and the underlying machine in the Jade view.


A question about using jpeg2000

Hello, I'm sorry to bother you, I have a question when I use this project about jpeg2000.  Should I download ffmpeg and openjpeg by myself? Thanks.

Yes, we used ffmpeg's implementation of jpeg2000 for our comparisons. You need to install it to use the evaluation scripts on your dataset.

Thanks for your answer! But just installing ffmpeg didn't work either. Does this project also need the OpenJPEG library compiled by C language? Or is my FFmpeg installation  in the wrong directory?  Sorry to bother you.

Please refer to https://ffmpeg.org/ffmpeg-codecs.html#jpeg2000 for ffmpeg

Hello, what are the versions of FFMpeg and JPEG2000. And the size of different pictures at the same compression level is the same

Hello, I am also unable to run JPEG2000. If it's convenient for you, could you please let me know which libraries I need to download? This would be a great help to me.


[Feature] Set default minimal height of event

Hi!
We've faced with a problem of displaying events with small duration or even with "negative" duration. "Negative" events it's events with wrong end date, when end is less then start (like start = 01.01.2024 10:00 and end = 01.01.2024 8:00). And we see, that solution is to set default minimal height of events, when calendar shows event, not strictly depending on duration.
Is it possible to implement?
We've found a place in event_group_controller to do that. In method _findOverlappingEvents() it's possible to add logic like that:
for (var event in overlappingEvents) {
    if (event.start.compareTo(event.end) >= 0 || event.end.difference(event.start).inMinutes < 30) {
      event = event.copyWith(dateTimeRange: DateTimeRange(start: event.start, end: event.start.add(Duration(minutes: 30))));
    }
    newOverlappingEvents.addAll(
      otherEvents.where(
        (otherEvent) {
          return ((otherEvent.start.isWithin(event.dateTimeRange) ||
              otherEvent.end.isWithin(event.dateTimeRange)) ||
              otherEvent.start == event.start ||
              otherEvent.end == event.end);
          },
      ),
    );
}

In this example we're change dateTimeRange of event if event is wrong (start > end) or when duration is less then 30 minutes (for short events).
We understand, that comparison event.end.difference(event.start).inMinutes < 30 is not correct in most cases and it's needed to adopt it with usage like heightPerMinute. But anyway, the idea should be clear.
What do you think?

Hi, yeah so that is something that works slightly different the calendar_rework branch (which is a rework of the current calendar package will release it when it is ready but you can try it out if you want).
You can just override the logic in the calculateHeight in the EventLayoutDelegate to force a minimum size.
Something like this
class CustomEventLayoutDelegate<Event> extends SideBySideLayoutDelegate<Event> {
  CustomEventLayoutDelegate({
    required super.events,
    required super.date,
    required super.heightPerMinute,
    required super.timeOfDayRange,
  });

  @override
  double calculateHeight(CalendarEvent<Event> event) {
    final result = super.calculateHeight(event);
    return result.clamp(30, double.infinity);
  }
}

You can even go so far as to define custom grouping logic by overriding the groupVerticalLayoutData
class CustomEventLayoutDelegate<Event> extends SideBySideLayoutDelegate<Event> {
  CustomEventLayoutDelegate({
    required super.events,
    required super.date,
    required super.heightPerMinute,
    required super.timeOfDayRange,
  });

  @override
  Iterable<HorizontalGroupData> groupVerticalLayoutData(
    Iterable<VerticalLayoutData> verticalLayoutData,
  ) {

    final horizontalGroups = <HorizontalGroupData>[];

    // Do some custom grouping with the events
    // and return the custom horizontalGroups.

    return horizontalGroups;
  }
}

Just note some of these names might change as the rework is not done yet.

Hi version 0.5.0 has been released which changed a few things with how this works, the new version now works as I described.


Integer valid values could be controlled by int::inRange

Could these values be better controlled with int::inRange
0,['Breast Cancer Allred Estrogen Receptor Score']
1,"['IRS Stage', 'Breast Cancer Allred Estrogen Receptor Score']"
10,['Breast Cancer Allred Estrogen Receptor Score']
2,"['IRS Stage', 'Breast Cancer Allred Estrogen Receptor Score']"
3,"['IRS Stage', 'Breast Cancer Allred Estrogen Receptor Score']"
4,"['IRS Stage', 'Breast Cancer Allred Estrogen Receptor Score']"
5,['Breast Cancer Allred Estrogen Receptor Score']
6,['Breast Cancer Allred Estrogen Receptor Score']
7,['Breast Cancer Allred Estrogen Receptor Score']
8,['Breast Cancer Allred Estrogen Receptor Score']

Not sure why 9 is not a valid value here.

Potential validation rules

Breast Cancer Allred Estrogen Receptor Score: int::inRange 0 10
IRS Stage: int::inRange 1 4

See also quoted integers in #173

Allred in breast cancer tier 3 - as of 2023.10.02 - no use of '10,' but 51 'unknown' and 0 null.
Agree to take no action.


Feature Request: Support Mozilla autoconfig

When using K9 mail with IMAP/SMTP servers other than well known mail service providers like GMail, automatic configuration of new mail accounts will almost certainly fail.
It would be great if K9 mail would support autoconfig like proposed by Mozilla: https://developer.mozilla.org/en-US/docs/Mozilla/Thunderbird/Autoconfiguration
Basically, this works as follows: I enter<EMAIL_ADDRESS>as email address. Instead of guessing that my mail server is imap.example.com, my connection security SSL/TLS at port 993 and my username fred, K9 contacts<EMAIL_ADDRESS>which returns an XML configuration file. This XML file tells K9, that the mail server is actually someotherhost.example.net, STARTTLS at port 143 is the preferred connection method and my user name is<EMAIL_ADDRESS>Microsoft uses similar techniques for their Exchange servers, but instead of querying autoconfig.example.com, the domain autodiscover.example.com is used. See https://msdn.microsoft.com/en-us/library/office/jj900169(v=exchg.150).aspx for more information.

Also note SRV records for configuring incoming mail servers
DNS-based configuration: created by IETF in 2011. SRV records give

all information for email configuration. Information about this way
can be found here: https://tools.ietf.org/html/rfc6186

and we could use MX records to determine SMTP server addresses.
This method doesn't completely autoconfigure it - you'd still have to know the login format - email address / user-part / something else.

It's a pity that the code from https://github.com/dzan/k-9/tree/experimental/settings-autocomplete-finalfixes was never merged.
Did it need extra work?

This should be a very very useful feature! Please, help us to promote selfhosting rather than just GAFAs!

Any progress with this? It works like a charm for Thunderbird and others and setting up a script which auto-creates the files on server side took me less than a day with parsing the postfix and dovecot configurations for proper values for all hosted domains and usernames. A static file is much less work again.
I'd like to see my favourite Android mail programm also support that easy mechanism. Many domain providers support this already out of the box.
Its as easy as download the file (replace example.com and fred with mail address):
<EMAIL_ADDRESS>or as second try
http://example.com/.well-known/autoconfig/mail/config-v1.1.xml
and parse the resulting XML:
The XML format is really easy:
https://developer.mozilla.org/en-US/docs/Mozilla/Thunderbird/Autoconfiguration/FileFormat/HowTo

P.S. The Microsoft methods are complicated, partly undocumented and not even supported by all Microsoft tools (I tested several when I tried to implement usable server support and in the end gave up), so the Thunderbird method is the only useful way ATM.

Actually there are at least three ways for autoconfiguration:

Autodiscover, xml based: created by Microsoft about 12 years ago. Probably mostly for Outlook/Exchange accounts, but also works with any other email account. This site should give all information needed for implementation:
Autoconfig, xml based: created by the Mozilla foundation. Quite simple xml configuration information originally created for Thunderbird, in the meantime also being used by other clients.
DNS-based configuration: created by IETF in 2011. SRV records give all information for email configuration.

With any of these configuration mechanisms a user would only have to provide his email address and a password to get the correct configuration settings for pop3, imap and smtp (submission).

As said in last comment the Microsoft stuff is very complicated, incompletely documented, differs between client versions and is not even fully supported by Microsoft tools, so I'd drop that in any consideration.
The SRV based stuff is fine, but limited in scope to Autoconfig. It can't provide username and some other details (login methods, preferred ports, ...).
Thought a fine implementation should probably try Autoconfig first and when not provided use SRV based configuration for the server names.

Supporting Mozilla autoconfig would be very important for self-hosted mail services... please, help us to fight GAFA spreading \o/.

This issue was worked on at GSOC 2017, see:

https://k9mail.github.io/2017/05/05/Google-Summer-of-Code-2017-Projects.html
https://daquexian.me/blog/2017/08/29/K9Mail_GSoC_summary/
https://github.com/k9mail/k-9/pull/2580

Unfortunately it has not been merged yet :-/

Any news for this very useful feature?

Any news?
BTW docs seem to be here: https://developer.mozilla.org/en-US/docs/Mozilla/Thunderbird/Autoconfiguration
There seem to be multiple ways that autodiscover is implemented.

It would be great if this feature were finally merged into k9. Bad enough that the android own mail client does not support any of these methods!

@smeinecke Last time I checked the branch that had this feature it had a lot of other changes so it was really hard to cherry-pick and the quality was not that great (but it worked! :) ).
I suppose it would take some time to get it in shape and at least support a minimal subset of autoconfig.xml.

So I added some SRV records as suggested in this thread for both imaps on port 993 and submission on 587, but K9 failed to detect these DNS records - forcing a manual configuration. The DNS records were correctly verified, so not sure why K9 doesn't use these.

Parts of the solution have been merged in march. If you care for one provider explicitly you can also add the config to providers.xml until this is resolved.

Hmm, my provider is altready in the XML, but in the last setup I did some time ago, I still had to configure it manually. Was a version with that already released?

Hi evreyone, I think that with the rapprochement between thunderbird and K9 this issue is again relevant?
Thank you in advance,

The roadmap for the near future includes improved account setup using Thunderbird account auto-configuration

It was even part of the official announcement :)

As a follow up to #6250 I would like to add to this thread that I have created a project tat allows you to run a server that supports all currently known autodiscover methods and the code has comments about what real apps use what method.
Feel free to leave feedback on https://github.com/wdes/mail-autodiscover-autoconfig
I hope it's not too much off topic
Anyway, it can help anyone that would want to test autodiscover code/feature

Thunderbird's Autoconfig mechanism has now been available in K-9 Mail 6.7xx beta versions for a while.


[QUERY] What happens if two Eventhub Trigger Functions use one EventHubs consumer group？

Library name and version
Microsoft.Azure.WebJobs.Extensions.EventHubs 5.0.0
Query/Question
Hi, team,
According to this doc, it says:

in certain scenarios, you may use up to five consumers or receivers per partition where all receivers get all the events of the partition.

But in this doc, it says:

Don't share consumer groups between function apps and other consumer applications. Each function app should be a distinct application with its own assigned consumer group to ensure offset integrity for each consumer and to simplify dependencies in an event streaming architecture. Such a configuration, along with providing each event hub-triggered function its own function app and storage account, helps set the foundation for optimal performance and scaling.

It seems if the total Eventhub Trigger Functions are no more than 5, it would be fine to let them use one consumer group. I don't know if I understand correct, please help explain~~ Thanks~
Environment
No response

Could you also please help explain, if an event hub trigger function instance undertakes excessive load, who will balance the work load?
And if the function instance just go down suddenly, who will discover and change this instance?

To answer your section question first:  Azure Functions has scaling behavior that will add or remove instances based on load.  It does this by measuring the number of events that are waiting to be read and adjusting as your Function does its processing.    A good place for information is the Target-based scaling documentation.  I would also recommend looking through the Best practices for reliable Azure Functions documentation.
For the premise of your original question - you do not want to configure different Functions to use the same consumer group.  The behavior will differ depending on what storage instance they're using:

If using the same storage instance, the two Functions would coordinate such that each owns a unique set of partitions.  Each function instance would read only from the partitions that it owns and would not see events from other partitions.

If not using the same storage instance, the two Functions would not be able to coordinate and would fight over partitions.  Since ownership would constantly cycle between them, you'd struggle to make forward progress and have extreme performance issues.


Add documentation on building custom components for apps and distributing as packages

It's not critical but some of the stuff in the article would benefit by https://github.com/posit-dev/py-shiny/pull/874 being merged so that the templates the user's would get when "playing along" match the article code more closely.

I tried building a JS wrapper and I have a few thoughts which are probably a bit bigger than this PR. So I pulled them into #48, I think this PR is a big improvement so we shouldn't block it with a big rewrite, but I think we do probably want to break this education task into smaller pieces down the road.


Adding sorting method

Looks good overall but I added some comments to sync with the rest of the package

See also: https://github.com/microbiome/OMA/issues/146

For example mia::calculateJSD have support
for both matrix and SE. Similarly, this could support TreeSE.

Support for SE should be enough for methods that do not use the tree information. Because TreeSE is SE (it inherits SE).
Currently we do not seem to have ordination methods that utilize the tree. Those exist I think but they are not commonly used. It is an interesting possibility but probably not very urgent.

So in addition to this current implementation, there could be method for TreeSE which has parameters for selecting

dimension reduction (dimred parameter)
number of dimensions (ncomponents)
whether to sort the output TreeSE (sort)

Adding dimension reduction would mean that we combine the steps or ordination and sorting? Possible but at leas it would complicate the implementation because now the sorting can be done based on any ordination method output. If these are combined, we need to choose which ordination methods are supported. If I got this right.
I am not sure if the radial theta angle methods is defined beyond the 2D case, hence ncomponents might not be valid option w.r.t. neatsort.
Sorting can be also done afterwards, the added value might be limited.

However, reducedDim slot requires that number of columns/samples match. If I understood correctly, this "neatsort" can be done also for features which means that reducedDim slot cannot be utilized (scater::calculatePCA and scater::calculateMDS can still be used as they have transposed parameter and they return only the output matrix). If that is the case, then I think that we should only support method for matrix to make things simpler.

Neatsort can be done for any Nx2 numeric matrix. Usually it is applied to sample scores is samples x PCaxes score matrix but in principle other uses are possible.
TBH it might not be critical to support feature loading matrices (features x PCaxes) because this doesn't seem very common. But it is possible and potentially useful, and out current implementations seems to have that support because it can operate with arbitrary Nx2 matrices.
Indeed, we could choose to simply support sample scores and use the reducedDim slot for that. This will complicate the implementation and the added value is not yet clear. I tend to think that we could keep getNearOrder support for matrices for now. If real use cases will call for a more streamlined implementation and TreeSE support we can always add that. But then we will need to address these more complex design questions.
This is somewhat related to the question on how we should present feature loadings in general in TreeSE objects because reducedDim is indeed only supporting the sample scores, and missing the feature loadings. The metadata slot can be used for that at least.

I meant that user could first do dimension reduction with methods that add the result to reducedDim and then call getNeatOrder(). As SE does not have reducedDim slot, at least SCE should have been supported.
But I agree, that only matrix should be supported at this point. This would make the usage simple
res <- calculatePCA(tse)
order <- getNeatOrder(res)
tse <- tse[ , order]
sechm(tse)

Right, I agree: getNearOrder could support reducedDim (TreeSE/SCE). But perhaps best to think about that later since the cost/benefit ratio seems a bit open.

Thanks both for the review and comments, I'm working on all of these updates today!

So I have implemented the changes but I have a few follow up questions just before I push.

In my example I've used the runPCA method to create the PCA matrix. This is because it was the only method I could see in OMA - (https://microbiome.github.io/OMA/docs/devel/pages/20_beta_diversity.html#sec-other-ord-methods). Is that OK?

Then also when calling sechm in my example am I right that we do the PCA, get the ordering from getNeatOrder, apply that to the tse object... but when we plot with sechm - do we plot using the z-transformed data?

When citing the publication, does that go in @details? Or @references? Or just slipped into the main description?

Please also see my responses to this OMA issue - https://github.com/microbiome/OMA/issues/146

More appropriate might be calculatePCA which returns matrix directly. We could highlight better where functions come from so that user can refer to those package manuals

Sounds correct. Z-transformation is usually applied to heatmaps since the colors are sensitive to scales. If there are features that varies a lot compared to others in "absolute" scale, those "others" look like that they do not vary at all. When values are scaled by features, the colors are scaled by feature (and not by whole dataset). If I remember correctly sechm has scale parameter, but it might be better to do manually (plot with Z-transformed data)

References https://github.com/microbiome/mia/blob/devel/R/calculateUnifrac.R

Noted. Leo probably knows better

I did not know that. Use scale(), but simplify the structure. There was at least one extra if-else

Run BiocCheck::BiocCheck(). There were some NOTEs related to these lines. Resolve those notes that can reasonably do. (If line contains only URL address, you can keep that even though it exceeds 80 characters)

Resolve errors in unit test

What're the errors - there was definitely no errors when I last checked

"All checks have failed" (see above Github runs)

test_check("miaViz")
[ FAIL 1 | WARN 6 | SKIP 1 | PASS 175 ]

══ Skipped tests (1) ═══════════════════════════════════════════════════════════
• miaTime cannot be loaded (1): 'test-2plotSeries.R:5:5'
══ Failed tests ════════════════════════════════════════════════════════════════
── Failure ('test-getNeatOrder.R:48:5'): Test check_args method ────────────────
.check_args(check_args_matrix[, c(1, 2)], "invalid_method") threw an error with unexpected message.
Expected match: "'arg' should be one of “mean”, “median”, “none”"
Actual message: "'arg' should be one of "mean", "median", "none""
Backtrace:
▆

├─testthat::expect_error(...) at test-getNeatOrder.R:48:5
│ └─testthat:::quasi_capture(...)
│   ├─testthat (local) .capture(...)
│   │ └─base::withCallingHandlers(...)
│   └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
└─miaViz:::.check_args(check_args_matrix[, c(1, 2)], "invalid_method")
└─base::match.arg(centering, c("mean", "median", "none"))

[ FAIL 1 | WARN 6 | SKIP 1 | PASS 175 ]
Error: Test failures
Execution halted
3 errors ✖ | 0 warnings ✔ | 4 notes ✖
Error: Process completed with exit code 1.
Run actions/upload-artifact@v3
/usr/bin/docker exec  01b1f8ffe51deb581bc7323b620ee9945bab51cd97bc196a49f233c188b27d66 sh -c "cat /etc/*release | grep ^ID"

Great work! Looks nice

Ubuntu check:
Error in eval(code, test_env): object 'expected_indices' not found
Mac and windows fails because they do not find new mia functions. They should be resolved automatically

Fixed the error. I also changed the allowed values for centering to be mean, median and NULL (NULL is common to denote that something has been disabled)

OK great, thanks, sounds good!


UnsolicitedResponse isn't Send

As reported in https://github.com/jonhoo/rust-imap/issues/250, the UnsolicitedResponse type isn't Send, but it should be. We should fix that.

Are you sure? rustdoc (and static_assertions) do show an Auto Trait implementation for Send and Sync on imap::types::UnsolicitedResponse.
Connection<T>, Client<T> and Session<T> do all also implement Send, however they are !Sync. However that's not because of UnsolicitedResponse but because std::mpsc::Sender and std::mpsc::Receiver are always !Sync

Huh, that's very weird indeed. In that case I don't know what was causing #250. There, @soywod claimed that mpsc::Sender<UnsolicitedResponse> and mpsc::Receiver<UnsolicitedResponse> weren't thread-safe, but they should be if the underlying type is Send (which it apparently is). @soywod can you speak more to how exactly the compiler was complaining at you?

Here the error I get:
error[E0277]: `std::sync::mpsc::Sender<UnsolicitedResponse>` cannot be shared between threads safely
   --> src/backend/imap/backend.rs:404:10
    |
404 | impl<'a> Backend for ImapBackend<'a> {
    |          ^^^^^^^ `std::sync::mpsc::Sender<UnsolicitedResponse>` cannot be shared between threads safely
    |
    = help: within `backend::imap::backend::ImapBackend<'a>`, the trait `Sync` is not implemented for `std::sync::mpsc::Sender<UnsolicitedResponse>`
    = note: required because it appears within the type `Session<ImapSessionStream>`

When I said "not thread-safe" I meant not Sync nor Send, and indeed std::mpsc::Sender is not Sync by definition.

Hi I am getting a similar error when trying to use it with relm4's sharedState type.
error[E0277]: `std::sync::mpsc::Sender<imap::types::UnsolicitedResponse>` cannot be shared between threads safely
  --> src/imap_state.rs:12:22
   |
12 | static IMAP_SESSION: SharedState<Arc<ImapSession>> = SharedState::new();
   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `std::sync::mpsc::Sender<imap::types::UnsolicitedResponse>` cannot be shared between threads safely
   |
   = help: within `imap_state::ImapSession`, the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Sender<imap::types::UnsolicitedResponse>`
   = note: required because it appears within the type `Session<StreamOwned<ClientConnection, TcpStream>>`
note: required because it appears within the type `ImapSession`
  --> src/imap_state.rs:7:10
   |
7  | pub enum ImapSession {
   |          ^^^^^^^^^^^
   = note: required for `std::sync::Arc<imap_state::ImapSession>` to implement `std::marker::Send`
   = note: required for `std::sync::RwLock<std::sync::Arc<imap_state::ImapSession>>` to implement `std::marker::Sync`
   = note: 1 redundant requirement hidden
   = note: required for `relm4::once_cell::imp::OnceCell<std::sync::RwLock<std::sync::Arc<imap_state::ImapSession>>>` to implement `std::marker::Sync`
   = note: required because it appears within the type `OnceCell<RwLock<Arc<ImapSession>>>`
   = note: required for `relm4::once_cell::sync::Lazy<std::sync::RwLock<std::sync::Arc<imap_state::ImapSession>>>` to implement `std::marker::Sync`
   = note: required because it appears within the type `SharedState<Arc<ImapSession>>`
   = note: shared static variables must have a type that implements `Sync`

error[E0277]: `std::sync::mpsc::Receiver<imap::types::UnsolicitedResponse>` cannot be shared between threads safely
  --> src/imap_state.rs:12:22
   |
12 | static IMAP_SESSION: SharedState<Arc<ImapSession>> = SharedState::new();
   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `std::sync::mpsc::Receiver<imap::types::UnsolicitedResponse>` cannot be shared between threads safely
   |
   = help: within `imap_state::ImapSession`, the trait `std::marker::Sync` is not implemented for `std::sync::mpsc::Receiver<imap::types::UnsolicitedResponse>`
   = note: required because it appears within the type `Session<StreamOwned<ClientConnection, TcpStream>>`
note: required because it appears within the type `ImapSession`
  --> src/imap_state.rs:7:10
   |
7  | pub enum ImapSession {
   |          ^^^^^^^^^^^
   = note: required for `std::sync::Arc<imap_state::ImapSession>` to implement `std::marker::Send`
   = note: required for `std::sync::RwLock<std::sync::Arc<imap_state::ImapSession>>` to implement `std::marker::Sync`
   = note: 1 redundant requirement hidden
   = note: required for `relm4::once_cell::imp::OnceCell<std::sync::RwLock<std::sync::Arc<imap_state::ImapSession>>>` to implement `std::marker::Sync`
   = note: required because it appears within the type `OnceCell<RwLock<Arc<ImapSession>>>`
   = note: required for `relm4::once_cell::sync::Lazy<std::sync::RwLock<std::sync::Arc<imap_state::ImapSession>>>` to implement `std::marker::Sync`
   = note: required because it appears within the type `SharedState<Arc<ImapSession>>`
   = note: shared static variables must have a type that implements `Sync`

For more information about this error, try `rustc --explain E0277`.
warning: `mail-client` (bin "mail-client") generated 1 warning
error: could not compile `mail-client` due to 2 previous errors; 1 warning emitted

Notable different is the reasoning here:
required because it appears within the type `Session<StreamOwned<ClientConnection, TcpStream>>`

Fixing the sender on the other hand would be possible with https://doc.rust-lang.org/std/sync/mpsc/fn.sync_channel.html however the same error would appear for the Receiver then

Hmm, as I understand the code the unsolicited_responses is really mostly used in a 'self-pipe' pattern, with Session always holding both the sending and receiving end and in essence using it as a FIFO queue. Especially given that right now the uses of unsolicited_responses_tx always take it as a &mut anyway, I do think it would make sense to use a VecDeque or plain Vec instead. That would also enable the multi-threaded access patterns that I think @soywod wants to work with.
Your thoughts @jonhoo? I'd be happy to write a PR for that change if you agree that it makes sense.

Ah, so the concern is actually that the client isn't Send/Sync (because it holds these senders/receivers), not that the elements of the channel aren't. That does make more sense! Yes, I think replacing it with a VecDeque makes a lot of sense, and would be happy to look at a PR!

I initiated a PR, please let me know if it resembles what you had in mind.

With the PR, I do not have anymore the cannot be shared between threads safely, but I still cannot run multiple tasks in parallel using a same Session. My intuition is that every session fn borrows self as mut &self mut which cannot be shared safely between threads without a Mutex. The PR has no impact on the initial problem.

Ah, yes, so, that's sort of fundamental to the IMAP protocol. It doesn't allow for multiple concurrent requests. So you'd need to either use a Mutex or spin up a separate Session for each client unfortunately.