Asynchronous execution and fetching field in parallel

Asynchronous execution is key point to ensure high performance of query, details in asynchronous-execution.
It seems that all the invoketion of @Controller handler method is serial by default. Will spring graphql support asynchronous execution of fetching field (probably similar to mvc-ann-async)?

I'm not sure I understand what you're missing. You can return Flux, Mono from controller methods.

@rstoyanchev Thanks for your response. Will spring-graphql support 'concurrent way' for the convenience of developers who prefer concurrent api.
reactor way in spring-graphql
I rewrite GreetingController#greeting to support fetching field asynchronously. If this is what your mean, (and this seems to be the only way for supporting execution asynchronously). Developers must be familiar with reactor api.
public class GreetingController {

	@QueryMapping
	public Mono<String> greeting() {
		return Mono.fromSupplier(() -> {
			RequestAttributes attributes = RequestContextHolder.getRequestAttributes();
			return "Hello " + attributes.getAttribute(RequestAttributeFilter.NAME_ATTRIBUTE, SCOPE_REQUEST);
		}).subscribeOn(Schedulers.parallel());
	}
}

concurrent way in graphql-java
Binding field with the async dataFetcher and assigning the Executor, instead the original dataFetcher.  graphql engine will fetching field in parallel.
public class AsyncDataFetcher<T> implements DataFetcher<CompletableFuture<T>> {

    /**
     * @param wrappedDataFetcher the data fetcher to run asynchronously
     * @param executor           the executor to run the asynchronous data fetcher in
     */
    public static <T> AsyncDataFetcher<T> async(DataFetcher<T> wrappedDataFetcher, Executor executor) {
        return new AsyncDataFetcher<>(wrappedDataFetcher, executor);
    }

    @Override
    public CompletableFuture<T> get(DataFetchingEnvironment environment) {
        return CompletableFuture.supplyAsync(() -> {
            //
        })
    }
}

concurrent way I expected
Be similar to mvc-ann-async:

Configure Executor( or AsyncTaskExecutor) in GraphQlSource.Builder;
If the return value type is Callable, and the dataFetcher will be decorate by AsyncDataFetcher#async.

Thanks for elaborating. I see now the perspective you're coming from.
A Callable is appealing but almost too simple. Once you make something asynchronous, it comes with a range of concerns. The more complete form of the Callable return value in Spring MVC is WebAsyncTask which also accepts a timeout, an Executor, along with error and completion callbacks. For all that this is a rather rudimentary mechanism that predates Reactor (as well as CompletableFuture for that matter) which provides options to compose further asynchronous stages, handle timeouts, and so on. This is before we even start talking about subscriptions and streaming. Spring MVC again has specialized types for SSE and response body streaming, again predating Reactor and Java 8.
In short, we've no plans to rebuild such purpose-built asynchronous facilities as we have in Spring MVC. These days both Spring MVC and Spring WebFlux support Reactor return values and that is the approach in Spring GraphQL from the start to make Reactor a core dependency, so you'll encounter it in all areas including client, server, and testing.
That said I see no reason why you couldn't reach a similar level of convenience with Reactor while also retaining the ability to compose multiple async operations together, deal with timeouts, and so on:
public class GreetingController {

	@QueryMapping
	public Mono<String> greeting() {
		RequestAttributes attributes = RequestContextHolder.getRequestAttributes();
		String name = attributes.getAttribute(RequestAttributeFilter.NAME_ATTRIBUTE, SCOPE_REQUEST);
		return executeAsync(() -> "Hello " + name);
	}

	private <T> Mono<T> executeAsync(Callable<T> callable) {
		return Mono.fromCallable(callable).subscribeOn(Schedulers.parallel());
	}

}

Can we use suspend in kotlin without any other frameworks?

Yes, I realize Reactor is a powerful and convenient tool. It is the first time that I spend lots of time studying Reactor, because I do encounter it in all areas of spring-graphql , as well as other spring document.
In fact, I am also familiar with graphql-java,(I used graphql-java for two years and fed back lots of code to graphql-java repository). This proposal is based on the reality of the use of graphql(java) and Reactor in China:

There are few developers who use Reactor, such as KuaiShou and MeiTuan.  Relying heavily on Reactor for synchronous execution will prevents developers in China from using spring-graphql, but will rarely promote the use of Reactor;

I always thought that It would be great that let developers focus on business logic without paying attention to Reactor/CompletableFuture( not write code about Mono/Flux/CompletableFuture). Asynchronous execution is implemented by @Configuration, @Bean and @Component.

Thank you for your elaborating. Maybe this issue should be discussed after spring-graphql has been more widely used and more feedback.

I think the currently implemetation is pretty well, as our internal GraphQL implementation which named TQL is the same which only need the deverper retun an Async or Reactive type, the the framework itself will lift it into the same reactive type. If the user want any parallelism , they can do themself, the framework itself runs on a dedicated threadpool.
We do have some @MaxBatchSize 、@Maxparallelism 、@Blocking annotation, but that's another stories.

@dugenkui03 think we could consider some simple steps to assist a data fetching controller method to become asynchronous.
For example considering that GraphQL Java expects CompletableFuture, we could turn a Supplier<?> return value from a controller method into CompletableFuture.supplyAsync(supplier, this.executor) along with some executor, configured centrally.
However, I would draw the line there. For anything more, e.g. want a timeout, or a different executor, just use CompletableFuture from the controller. Similarly if you are integrating with some asynchronous API and need something more like like Spring MVC's DeferredResult, I would say just use CompletableFuture agian.
What do you think?

@hepin1989 if we support Supplier<?> as a return value that would be similar to the idea of an @Blocking annotation.
@pinguinjkeke you can use suspend but Reactor is still a required dependency and used in our APIs where contracts need to be asynchronous or where we need to compose asynchronous logic. This is comparable to WebFlux.

@rstoyanchev @hepin1989  Thanks for your replay.
Maybe I am really outdate, and we should use Reactor just like CompletableFuture.

We could turn a Supplier<?> return value from a controller method into CompletableFuture.supplyAsync(supplier, this.executor) along with some executor, configured centrally.

Simple but useful method, you may think that it is not elegant, such as timeout problem and thread management. But in brief, I think support Supplier(or others) is better than just Reactor. The matter is that how to support flexibly.

@rstoyanchev thanks for the answer. It will be a good push for spring-graphql if we have Kotlin example which shows how it's simple to start.


this project is not working in android 9 and  android 10 versions.

Hi Munnichoudhary,
it works with previous versions because the Camera API changed with new ones to Camera API 2 and the coding would need more improvements


Full N64 color space and resolution with the just announced Pico 2?

I wonder how much work it would be to port this over to the new Pico 2. https://www.raspberrypi.com/products/rp2350/
The Pico 2 has built-in 520 KiB RAM. This is nearly double what the first Pico came with.
Currently PicoDVI-N64 works with a 320x240 internal framebuffer with up to rgb565 color depth.
The N64 can output up to 640x480 with rgb777.
Is it feasible to actually handle 7 bits per color? This would mean that each pixel consumes 21 bits which does not fit into the 8 bits raster. Assume this is possible and we only want to support 240p content. This would result in 395 KiB memory usage and so could fit into the Pico 2 RAM.
And then I found this one: https://shop.pimoroni.com/products/pimoroni-pico-plus-2
The new RP2350 seems to have external RAM support and this variant comes with 8MB of additional RAM. So the full sized frame buffer should fit with ease.
So long story short: I don't know if it would be possible at all but if this project can be extended to full color space and resolution, this would be a really interesting low cost HDMI mod. Currently (in my opinion) it is a nice demo to show what is possible with the RP2040 but the native resolution of the N64 already is not that great on big HDMI screens. Further limiting this doesn't really improve things.

https://m.youtube.com/watch?v=kQzxHdXNY6g

The N64 can output up to 640x480 with rgb777.

+1
I would love to see if this is possible with Pico 2!

The N64 can output up to 640x480 with rgb777.

Yeah, it'd be awesome to have full color output. Also, a switch to select between 640 and 320 modes depending on the content would be nice. And, a native 320x240 output mode would be great for scalers such as the RT4K and the OSSC Pro to achieve lagless integer scaling to 4K. I would also suggest implementing a 192KHz audio mode for better quality if possible with the Pico 2.


[ACR] Add a new argument '--expose-token' for 'az acr login'

Add a new argument '--expose-token' for 'az acr login' to expose access token.
Update help manual accordingly.

Close #9442
Behavior:
[specify --expose-token]

[docker command error]

[help manual]

History Notes:
(Fill in the following template if multiple notes are needed, otherwise PR title will be used for history note.)
[Component Name 1] (BREAKING CHANGE:) (az command:) make some customer-facing change.
[Component Name 2] (BREAKING CHANGE:) (az command:) make some customer-facing change.

This checklist is used to make sure that common guidelines for a pull request are followed.

[ ] The PR title and description has followed the guideline in Submitting Pull Requests.

[ ] I adhere to the Command Guidelines.

add to S165.


CreateStartScripts task generates classpath in wrong order

The transformation of the startscripts' classpath property to the relative classpath used inside the start scripts does not maintain the classpath order.
Expected Behavior
Relative classpath must have the same order as the configured classpath
Current Behavior
getRelativeClasspath() uses
public static <R, I> Set<R> collect(Set<? extends I> set, Transformer<? extends R, ? super I> transformer) {
        return collect(set, new HashSet<R>(), transformer);
}

which uses a new HashSet instance to create the transformed Iterable and therefore the order is undefined.
CreateStartScripts.java
Steps to Reproduce (for bugs)
Bug is still on master, discovered in version 4.0.1

Good find @thhalm.  Would you be interested in contributing a fix?

Sure, I will create a pull request.
btw: I think only version >= 4.0 are affected. (in 3.5. it was still correct as com.google.common.collect.Iterables#transform was used)


Error "Class DOMDocument not found"

Pint Version: 0.1.2
PHP Version: 8.0.20

Running using Laravel sail with Docker.
Description:
I get an error when running ./vendor/bin/pint. It prints out a couple of dots and checks, and then shows an exception:
./vendor/bin/pint -v

  ...........................................................................................
In HtmlRenderer.php line 32:

  [Error]
  Class "DOMDocument" not found

This is the full trace with pint -v
Exception trace:
  at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/nunomaduro/termwind/src/HtmlRenderer.php:32
 Termwind\HtmlRenderer->parse() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/nunomaduro/termwind/src/HtmlRenderer.php:24
 Termwind\HtmlRenderer->render() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/nunomaduro/termwind/src/Functions.php:41
 Termwind\render() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/app/Output/SummaryOutput.php:62
 App\Output\SummaryOutput->handle() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/app/Actions/ElaborateSummary.php:47
 App\Actions\ElaborateSummary->App\Actions\{closure}() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/illuminate/support/helpers.php:302
 tap() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/app/Actions/ElaborateSummary.php:47
 App\Actions\ElaborateSummary->execute() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/app/Commands/DefaultCommand.php:55
 App\Commands\DefaultCommand->handle() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/app/Providers/CommandsServiceProvider.php:32
 App\Providers\CommandsServiceProvider->App\Providers\{closure}() at n/a:n/a
 call_user_func() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/illuminate/container/Container.php:348
 Illuminate\Container\Container->callMethodBinding() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/illuminate/container/BoundMethod.php:90
 Illuminate\Container\BoundMethod::callBoundMethod() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/illuminate/container/BoundMethod.php:37
 Illuminate\Container\BoundMethod::call() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/illuminate/container/Container.php:651
 Illuminate\Container\Container->call() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/illuminate/console/Command.php:136
 Illuminate\Console\Command->execute() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/symfony/console/Command/Command.php:291
 Symfony\Component\Console\Command\Command->run() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/illuminate/console/Command.php:121
 Illuminate\Console\Command->run() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/symfony/console/Application.php:998
 Symfony\Component\Console\Application->doRunCommand() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/symfony/console/Application.php:299
 Symfony\Component\Console\Application->doRun() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/symfony/console/Application.php:171
 Symfony\Component\Console\Application->run() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/illuminate/console/Application.php:102
 Illuminate\Console\Application->run() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/laravel-zero/foundation/src/Illuminate/Foundation/Console/Kernel.php:129
 Illuminate\Foundation\Console\Kernel->handle() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/vendor/laravel-zero/framework/src/Kernel.php:96
 LaravelZero\Framework\Kernel->handle() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/app/Kernel.php:19
 App\Kernel->handle() at phar:///home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint/pint:36
 require() at /home/laura/PhpstormProjects/rm-notesync/vendor/laravel/pint/builds/pint:14
 include() at /home/laura/PhpstormProjects/rm-notesync/vendor/bin/pint:120

Steps To Reproduce:
All I did was run pint -v, the project I'm working on is not open-source so I can't link that unfortunately.

Looks like the installation is missing php-dom, is this an issue with sail? Is this an extension that can be specified in composer.json like ext-zip, ext-json etc?

I've added the missing extension: 43989e8.
Yet, tomorrow I will investigate the issue in Sail.

Ah, I think the only issue was the missing dependency in composer.json.
I accidentally ran the command outside of my container, rather than inside. Within sail it runs well, and the xml extension is loaded.

Thank you for the feedback.


TypeError: Can't modify immutable headers. in handleDataRequestRR (Cloudflare Pages)

Reproduction
Go to https://github.com/ilyasotkov/polished-truth-b001, open in Codespaces or local devcontainer, follow the steps in README.
This pull request https://github.com/remix-run/remix/pull/6783 is the culprit, present since 1.19.0, worked fine before that. The error seems quite baffling to me because the code below is so basic, I can't imagine any useful Remix app not doing something like this. Am I missing something here? Any workarounds?
import { Form } from '@remix-run/react';

export const loader = async () => {
  const r = new Request(`https://httpbin.org/get`);
  return await fetch(r);
};

export const action = async () => {
  return null;
};

export default function ButtonForm() {
  return (
    <div>
      <Form method="post">
        <button type="submit">Send Request</button>
      </Form>
    </div>
  );
}

System Info
System:
    OS: Linux 6.2 Debian GNU/Linux 12 (bookworm) 12 (bookworm)
    CPU: (2) x64 AMD EPYC 7763 64-Core Processor
    Memory: 5.77 GB / 7.74 GB
    Container: Yes
    Shell: 5.2.15 - /bin/bash
  Binaries:
    Node: 18.20.1 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.5.0 - /usr/local/bin/npm
  npmPackages:
    @remix-run/cloudflare: ^2.8.1 => 2.8.1
    @remix-run/cloudflare-pages: ^2.8.1 => 2.8.1
    @remix-run/dev: ^2.8.1 => 2.8.1
    @remix-run/react: ^2.8.1 => 2.8.1
    vite: ^5.1.0 => 5.2.8

### Used Package Manager

npm

### Expected Behavior

No error on button press.

### Actual Behavior

✘ [ERROR] TypeError: Can't modify immutable headers.

      at handleDataRequestRR
  (file:///projects/apps/polished-truth-b001/node_modules/@remix-run/server-runtime/dist/server.js:158:22)
      at async requestHandler
  (file:///projects/apps/polished-truth-b001/node_modules/@remix-run/server-runtime/dist/server.js:94:18)
      at async handleFetch
  (file:///projects/apps/polished-truth-b001/node_modules/@remix-run/cloudflare-pages/dist/esm/worker.js:75:18)
      at null.<anonymous> (async
  file:///projects/apps/polished-truth-b001/.wrangler/tmp/dev-CQLC77/functionsWorker-0.8639886703431474.js:6055:14)
      at async next
  (file:///projects/apps/polished-truth-b001/node_modules/wrangler/templates/pages-template-worker.ts:161:22)
      at async Object.fetch
  (file:///projects/apps/polished-truth-b001/node_modules/wrangler/templates/pages-template-worker.ts:180:11)
      at async jsonError
  (file:///projects/apps/polished-truth-b001/node_modules/wrangler/templates/middleware/middleware-miniflare3-json-error.ts:22:10)
      at async drainBody
  (file:///projects/apps/polished-truth-b001/node_modules/wrangler/templates/middleware/middleware-ensure-req-body-drained.ts:5:10)
      at async jsonError
  (file:///projects/apps/polished-truth-b001/node_modules/wrangler/templates/middleware/middleware-miniflare3-json-error.ts:22:10)
      at async drainBody
  (file:///projects/apps/polished-truth-b001/node_modules/wrangler/templates/middleware/middleware-ensure-req-body-drained.ts:5:10)

[wrangler:inf] GET / 500 Internal Server Error (136ms)

@ilyasotkov this will probably fix it for you https://github.com/remix-run/remix/pull/8722/files

@arjunyel I just checked, and it doesn't fix it. Also I don't see how the changes in your PR are anyhow related to the PR that introduced the issue: https://github.com/remix-run/remix/pull/6783.
This issue is about HMR returning 500 because workerd is throwing Can't modify immutable headers. I can work around it (with significant UX regression) by doing this
- <Form method="post">
+ <Form method="post" reloadDocument>

I've created a simple test project  that reproduces this error in the context of error handling: it used to be possible to throw a Response object out of a loader or action function and have that response be represented in the output of useRouteError. Now, it seems that the handleDataRequest function is introducing an error because it is calling headers.set on a header collection with an immutable guard. This generates the "Can't modify immutable headers" error, which pollutes the error handling. We're currently working around this problem by re-building Response objects without headers before we throw them, but it would be helpful to have the root cause corrected.
Here's sample output from the test project:
at _Headers.set (node:internal/deps/undici/undici:2166:17)
    at handleDataRequest (/Users/pkch/sandbox/remix_action_defect/my-remix-app/node_modules/@remix-run/server-runtime/dist/server.js:182:21)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async requestHandler (/Users/pkch/sandbox/remix_action_defect/my-remix-app/node_modules/@remix-run/server-runtime/dist/server.js:100:18)
    at async nodeHandler (/Users/pkch/sandbox/remix_action_defect/my-remix-app/node_modules/@remix-run/dev/dist/vite/plugin.js:841:27)
    at async /Users/pkch/sandbox/remix_action_defect/my-remix-app/node_modules/@remix-run/dev/dist/vite/plugin.js:844:15

Thank you for this issue. I ran into this some time ago after upgrading Remix and thought maybe I was wrong.  I was so reproducing this in basically the way, which I don't see as being "bad." I was going to end up doing something like cloning the response or just building and throwing a new one to avoid it. Hopefully this gets fixed

https://github.com/remix-run/remix/pull/9693#issuecomment-2218274450

This is a duplicate of https://github.com/remix-run/remix/issues/9691 and should be resolved in Remix 2.10.2 via https://github.com/remix-run/remix/pull/9693.  Please let us know if not!

This is a duplicate of #9691 and should be resolved in Remix 2.10.2 via #9693. Please let us know if not!

I've updated my error handling repeatable case to 2.10.2 and the problems seems to still be present. I realize the context of this example is different than the specifics of this issue, and I would be happy to open a separate issue if that would be helpful.

It looks like you added the (unused) remix dependency at 2.10.2 - but forgot to update the others: https://github.com/pkpjpm/remix_action_defect/blob/main/my-remix-app/package.json#L14.  They all need to be updated in lock-step.
I actually created a utility to do this a while back to make it easier: https://github.com/brophdawg11/upgrade-remix
I can confirm that once all are updated the problem is resolved 👍

Bravo, that's done it! Thanks very much for your help


[MM-21000] Fix using local storage to return to a DM/GM

Summary
Fix whenever using local storage to return to a previous DM/GM as we were only looking for channels.
Ticket Link
MM-21000

@hmhealey added, thanks for the suggestion :)

Creating a new SpinWick test server using Mattermost Cloud.

Mattermost test server created! :tada:
Access here: https://mattermost-webapp-pr-4791.test.mattermost.cloud

Account Type
Username
Password

Admin
sysadmin
Sys@dmin123

User
user-1
User-1@123

Test server destroyed

/update-branch

Will try to auto merge this PR once all tests and checks are passing. This might take up to an hour.

Trying to auto merge this PR.

Pull Request successfully merged
SHA: 6dee486d4bbfa34823e3ac0ebd194fba1d5a2c19


Add the structural similarity (SSIM) index metric as a built-in loss operation

In many cases existed built-in losses in TensorFlow do not satisfy needs.  We can add ssim or (1-ssim) as the loss function into TensorFlow.
There is existed solution provided on StackOverflow, but it is better to have the built-in function with fully covered unit tests.

Thank you for your post. We noticed you have not filled out the following field in the issue template. Could you update them if they are relevant in your case, or leave them as N/A? Thanks.
Have I written custom code
OS Platform and Distribution
TensorFlow installed from
TensorFlow version
Bazel version
CUDA/cuDNN version
GPU model and memory
Exact command to reproduce

One thing to note is that you can get different values depending on implementation details, which is not good when reporting numbers in papers. Here is yet another implementation, which I based it off scikit-image's compare_ssim implementation (and the numbers match): https://gist.github.com/alexlee-gk/cbc9bfa6e5be51b53c622684cec0a3f3

Hi, I converted the numpy version of ms-ssim in tensorflow-contrib to a TensorFlow version. I could add it to tensorflow and maintain it, but I've never contributed to tensorflow before. I've looked at the contributing.md, so the procedure is clear to me. I just don't know in which file to put it. I see there are losses in Tensorflow contrib, but it seems depreciated. Should I still put it in there? Thanks for the help.
Here is the reference implementation that I used, it seems based on the matlab reference implementation of the author of the MS-SSIM.
https://github.com/tensorflow/models/blob/master/research/compression/image_encoder/msssim.py
Please tell me if you're willing to have the MS-SSIM build-in in tensorflow, and if yes, in which file I should put it. I'll do a proper PR then.

Any chance that those functions can support 3D (volumetric) data ?

@cancan101 have you found a solution to your problem?


add gbr to pending wallet reports & update perview for 1:1 reports

Details

This PR fixes the issue with not showing GBR for the report that require user action to complete create wallet and as well to show correct preview message for the 1:1 chat instead of showing system message.

https://github.com/Expensify/App/issues/30033
https://github.com/Expensify/App/issues/30037

Fixed Issues

$ https://github.com/Expensify/App/issues/30037
PROPOSAL: https://github.com/Expensify/App/issues/30037#issuecomment-1772395813
$ https://github.com/Expensify/App/issues/30033
PROPOSAL: https://github.com/Expensify/App/issues/30033#issuecomment-1772477889
Tests

[x] Verify that no errors appear in the JS console

Open App request any amount form any user (Silver Wallet)
Observe the 1:1 chat and IOU report both should show owes message.
Pay money using expensify method from User B (gold wallet)
For User B there shouldn't be no GBR
For User A there should GBR on IOU report
For 1:1 the preview should paid message
For IOU report the message should contain paid & amount held system message both should be there

Offline tests

Same as above
QA Steps

[x] Verify that no errors appear in the JS console
Same as above

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native
https://github.com/Expensify/App/assets/59088937/ecb396d9-9e27-4b6a-ad4a-adf61bf75b9f

Android: mWeb Chrome
https://github.com/Expensify/App/assets/59088937/46bdcbeb-a8fb-43ed-9d47-25e55f4f5c70

iOS: Native
https://github.com/Expensify/App/assets/59088937/0fd8f630-6bdd-4415-acd4-0d52688643ea

iOS: mWeb Safari
https://github.com/Expensify/App/assets/59088937/fc72f143-7b35-4182-b7d0-b0fbbe03ccfe

MacOS: Chrome / Safari
https://github.com/Expensify/App/assets/59088937/9f60e8ca-2c8c-4f5b-89f6-4b1da06f23ca

MacOS: Desktop
https://github.com/Expensify/App/assets/59088937/d9683d1c-3893-41c4-97b6-e99add24755c

@situchan @techievivek PR is up.!

@b4s36t4 I think that's a good test case. We can update it to be compatible for our change.

Also, please watch https://github.com/Expensify/App/pull/30079 and pull main after it's merged.

Agree with @situchan let's update the test

@b4s36t4 Yes, let's please update the tests as per new changes rather than getting rid of it.

@situchan Can you please give this a review, I will also test this on my end.

@techievivek unit test failing

https://github.com/Expensify/App/assets/59088937/b5308d52-428b-4bf0-8135-996c59c6a5eb

With both checks for expense & chat reports (1:1 & IOU thread)

How about introducing new function called isPaid?
So isSettled = isPaid && isWaitingOnBankAccount === false

How about introducing new function called isPaid?
So isSettled = isPaid && isWaitingOnBankAccount === false

The issue here is that for DM chat report that contains the IOU preview we want to show the settled message in LHN even if there is isWaitingOnBankAccount property set on the IOU report, but for the IOU report, we want to show the system message that we are waiting for the payee to enable their wallet/bank account.

We can keep the isSettled logic unchanged and then we can add an OR operator to handle the above case.
if (isSettled(report.reportID) || (report.isWaitingOnBankAccount && isPreviewMessageForParentChatReport)) {
    // Show the paid message
}

isSettled is used in many places. i.e. MoneyRequestHeader
I believe we should not modify any request if paid and isWaitingOnBankAccount.
But as seen here, it's still editable:
https://github.com/Expensify/App/blob/e73cd31a567c5f026f3a9f1435ecb6f99b2fae82/src/pages/EditRequestPage.js#L118-L122

I believe we should not modify any request if paid and isWaitingOnBankAccount.

Yeah, I agree. Even if we're not adding this right now, we should update it as soon as possible.

@situchan @techievivek Updated the condition here https://github.com/Expensify/App/pull/30090/files#diff-65c096044d5f69b35bcdec14c99c4fda4580759df9b1a7c36650d58eea276f1dR1682-R1683
Sorry for back&forth changes :)

This is minor inconsistency:
Started settling up, payment is held until ${submitterDisplayName} enables their Wallet
started settling up, payment is held until ${submitterDisplayName} adds a bank account

Can we lowercase first letter for wallet message?

@situchan lowercase for wallet message or upper case for bank message? @techievivek for review :)

@situchan lowercase for wallet message or upper case for bank message? @techievivek for review :)

I believe lowercase. All system messages already do.

Reviewer Checklist

[x] I have verified the author checklist is complete (all boxes are checked off).
[x] I verified the correct issue is linked in the ### Fixed Issues section above
[x] I verified testing steps are clear and they cover the changes made in this PR

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[x] I checked that screenshots or videos are included for tests on all platforms
[x] I included screenshots or videos for tests on all platforms
[x] I verified tests pass on all platforms & I tested again on:

[x] Android / native
[x] Android / Chrome
[x] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[x] I verified proper code patterns were followed (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I verified that this PR follows the guidelines as stated in the Review Guidelines
[x] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
https://github.com/Expensify/App/assets/108292595/18078f5d-4e4b-4a9b-a75f-eff5fcf9e0c2
https://github.com/Expensify/App/assets/108292595/dec16eb5-6b0f-490b-aa89-b456a5ca2a01

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

I just noticed that LHN last message is wrong. It says "paid elsewhere"

@situchan  Checking.

The RCA of the above bug is that we don't add the PAY IOU action on the expense report if the payee don't have a linked bank account: https://github.com/Expensify/Auth/blob/40dc46bab88a5c1a45edf0b2e4cdd02808ca59e1/auth/command/PayMoneyRequest.cpp#L291, as a result we are unable to fetch the correct report action that would tell us the correct payment type.

CC @MariaHCD @mountiny please have a look at the above comment https://github.com/Expensify/App/pull/30090#issuecomment-1775032232 the flow here is significantly different from the PayWithWallet flow. (Honestly speaking there is a lot of cleanup things that are there on the flow and maybe we should revisit the whole money request/pay flow and make it consistent).

@situchan Please check now, I have fixed the paid elsewhere issue.

@situchan Can we have another review here, please? 🙏

So far looks good against ngrok server.
I will re-point to production server and retest IOU, expense request with elsewhere pay (without wallet)

I'm seeing GBR even though I am not payer but payee. I have nothing to pay

@techievivek I dont necessarily think that is a problem (though I agree that the fact these two flows are inconsistent is not good, the wallet flow was added as second as the reimbursement flow already existed in the oldDot)
I think we should mainly not be showing "paid this and this" when the money actually has not left their account yet, nothing will happen until the employee adds the bank account and in that case its fine there is no Pay action and the alternate text should more so talk about hte fact the user needs to add a bank account imho

Failing test looks related to changes:

@Julesssss fixed.

@mountiny

I dont necessarily think that is a problem (though I agree that the fact these two flows are inconsistent is not good, the wallet flow was added as second as the reimbursement flow already existed in the oldDot)

My primary concern is how some of our code, particularly on the frontend, tries to follow the same code for both flows. At the same time, in the backend, we have different commands handling this differently, resulting in inconsistencies. For e.g. in the case when the user needs to add a bank account or wallet. So we should either try to make it consistent or handle it distinctly.
I think we can re-look into the code(both frontend and backend) and try to clean this up once the urgency has settled a bit.

I think we can re-look into the code(both frontend and backend) and try to clean this up once the urgency has settled a bit.

I agree

Elsewhere flow looks good.
Currently retesting with gold wallet flow as GBR condition changed

@situchan let me know in slack once you finished, thanks!

@b4s36t4 did you retest gold wallet flow on payer side? works fine on payee side.
@techievivek stopped ngrok so not able to test.

Yes, on payer side it will show GBR till user pays, it will hide after payment done.

payer side is verified when connected to ngork, payee side verified without ngrok (ngrok is slow & taking long to get response)

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 1.3.89-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes.

🚀 Deployed to production by https://github.com/marcaaron in version: 1.3.89-6 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

🚀 Deployed to staging by https://github.com/mountiny in version: 1.3.91-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/marcaaron in version: 1.3.91-8 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


Revert "New add Baichuan Model"

Reverts langchain-ai/langchain#11714
This has linting and formatting issues, plus it's added to chat models folder but doesn't subclass Chat Model base class

cc @cloudscool

Hi，@nfcampos，Because Baichuan currently does not have an SDK, I will temporarily use the official simplest method. Afterwards, I will continuously update it according to the situation until it is better integrated into Langchain.


Fully Functional Specs.

Coverage decreased (-4.4%) to 78.91% when pulling cc202ad25c4a9d4320f457e3a069c206ac988a1d on adjavaherian:master into 9cabcf94c83ceb029f0782d9067cd78a7e065966 on danvk:master.


fix: set merge conflict Action to run every 12 hours instead of every 15 minutes

Checklist:

[x] I have read freeCodeCamp's contribution guidelines.
[x]  My pull request has a descriptive title (not a vague title like Update index.md)
[x] My pull request targets the master branch of freeCodeCamp.
[x] All the files I changed are in the same world language, for example: only English changes, or only Chinese changes, etc.

Related to PR #39327
After a discussion with @ojeytonwilliams, we decided that running this Action every 12 hours is sufficient for labeling/unlabeling  merge conflicts.

@ojeytonwilliams Sorry about that crontab mistake.

Nothing to worry about.  Even if I'd missed it, it wouldn't have been a big problem.


Update copyright year to 2016

Updated copyright year to 2016 in some files where it was still written as 2015

@DennisLfromGA @dnschneid , any word on review of this PR?

Sorry @mmattioli, I'm just a crouton enthusiast and not a developer so I can't approve or merge it.

Are people jostling for the coveted copyright spot in git blame?  I guess we'll see who is first to submit a copyright PR in 2017 :)
Anyway, thanks for reminding me which year it is.


hint appears twice

When running on API 23, hint is displayed twice when using android:entries in xml.
please include this fix.  https://github.com/ganfra/MaterialSpinner/pull/49

Thanks!


Loading a default schema fails on Windows 10 with Python 3.7

Here's my full error trace.
Traceback (most recent call last):
  File "utils.py", line 534, in <module>
    xml_to_csv("test_data/new_and_updated.xml")
  File "utils.py", line 414, in xml_to_csv
    v203_schema = iati.default.activity_schema('2.03')
  File "C:\Program Files\Python37\lib\site-packages\iati\version.py", line 365, in wrap_decimalise_integer
    return input_func(version, *args[1:], **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\iati\version.py", line 386, in wrap_normalise_decimals
    return input_func(version, *args[1:], **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\iati\version.py", line 303, in wrap_allow_known_version
    return input_func(*args, **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\iati\default.py", line 311, in activity_schema
    return _schema(iati.resources.get_activity_schema_paths, iati.ActivitySchema, version, populate)
  File "C:\Program Files\Python37\lib\site-packages\iati\default.py", line 288, in _schema
    schema = _populate_schema(schema, version)
  File "C:\Program Files\Python37\lib\site-packages\iati\default.py", line 255, in _populate_schema
    codelists_to_add = codelists(version)
  File "C:\Program Files\Python37\lib\site-packages\iati\default.py", line 127, in codelists
    return _codelists(version)
  File "C:\Program Files\Python37\lib\site-packages\iati\version.py", line 365, in wrap_decimalise_integer
    return input_func(version, *args[1:], **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\iati\version.py", line 386, in wrap_normalise_decimals
    return input_func(version, *args[1:], **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\iati\version.py", line 271, in wrap_allow_fully_supported_version
    return input_func(*args, **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\iati\default.py", line 101, in _codelists
    paths = iati.resources.get_codelist_paths(version)
  File "C:\Program Files\Python37\lib\site-packages\iati\version.py", line 365, in wrap_decimalise_integer
    return input_func(version, *args[1:], **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\iati\version.py", line 336, in wrap_allow_possible_version
    return input_func(*args, **kwargs)
  File "C:\Program Files\Python37\lib\site-packages\iati\resources.py", line 99, in get_codelist_paths
    files = pkg_resources.resource_listdir(PACKAGE, folder_path[len(resource_filesystem_path('')):])
  File "C:\Program Files\Python37\lib\site-packages\pkg_resources\__init__.py", line 1155, in resource_listdir
    resource_name
  File "C:\Program Files\Python37\lib\site-packages\pkg_resources\__init__.py", line 1417, in resource_listdir
    return self._listdir(self._fn(self.module_path, resource_name))
  File "C:\Program Files\Python37\lib\site-packages\pkg_resources\__init__.py", line 1510, in _listdir
    return os.listdir(path)
FileNotFoundError: [WinError 3] The system cannot find the path specified: 'C:\\resources\\standard\\2-03\\codelists'

Should be reproducible with just the code in the README:
import iati.default

schema = iati.default.activity_schema('2.03')

Seems to be an issue with absolute filepaths?

Looks like all paths generated to resources are relative:
https://github.com/IATI/pyIATI/blob/269a2c1cb73479875a534ae3dfcb6ef9c03e1766/iati/resources.py#L28-L31
There are certainly situations where this will not do the right thing.

I think you are right that this is due to filepaths. Specifically, I think it’s because resources are all loaded via relative filepaths:
https://github.com/IATI/pyIATI/blob/269a2c1cb73479875a534ae3dfcb6ef9c03e1766/iati/resources.py#L28-L31
I think best practice in packages is to use absolute filepaths everywhere. I guess that’s the problem here.

Oddly enough it works just fine on my Ubuntu partition and doesn't try and access /resources/ at my system root. Just a Windows specific bug for the moment.

Yes, same. I think using relative URLs in this way is dodgy, because it’s not always obvious what it is relative to. That’s why package developers usually do something like:
basepath = os.path.dirname(os.path.abspath(__file__))

…and then prepend that to all relative paths.

Aside: contrary to what the usage section in the README says, this repo is not currently under active development.
See this announcement on IATI discuss: https://discuss.iatistandard.org/t/state-of-pyiati/1360/7

Found a fix, in case this project is ever rebooted.
On line 99 here,
https://github.com/IATI/pyIATI/blob/269a2c1cb73479875a534ae3dfcb6ef9c03e1766/iati/resources.py#L99
folder_path[len(resource_filesystem_path('')):] comes out as "\\resources\\standard\\2-03\\codelists" on Windows, while it comes out as "/resources/standard/2-03/codelists" on Unix based operating systems.
For some reason, the default Python package resource manager pkg_resources interprets that as relative to the package for Unix, but absolute to the C: drive for Windows. Can be fixed by importing os, splitting the path, and re-joining it before searching for the resources.
import os
files = pkg_resources.resource_listdir(PACKAGE, os.path.join(*folder_path[len(resource_filesystem_path('')):].split(os.sep)))

I've added a PR to fix this in #329.
I've also separately added python 3.7 builds to Travis in #330 - ideally you would have a seperate Travis builds running in both Unix and Windows to confirm cross-platform support, but I haven't looked into if/how that is possible.

ideally you would have a seperate Travis builds running in both Unix and Windows to confirm cross-platform support, but I haven't looked into if/how that is possible.

It’s not currently. Travis says:

The language 'python' is currently unsupported on the Windows Build Environment.


Feature: access mobile-specific features

The aim is to be able to access mobile-specific features when deploying the app on mobile.
For instance:

Setting up an alarm
Updating the Calendar
Accessing files
Accessing GPS data

Mobile-specific features might be accessible from existing PyQt5 modules:

QtBluetooth -> Access Bluetooth features (as mentioned in https://github.com/achille-martin/simple-pyqt5-android-app/issues/15)
QtMultimedia -> Access camera and audio features
QtNetwork -> Access Networking features (Wifi and HTTP?)
QtPositioning -> Access GPS features
QtSensors -> Access available hardware sensors
QtWebSockets -> Access web socket features to connect to servers

Tips are given on kviktor's github page on how to deal with Android permissions specifically.

This issue will be investigated in the Basic+ plan.


fix(bug): Resolve dependency conflict between KAG and openspg-knext

fix issue: #428

I have read the CLA Document and I hereby sign the CLA


Evaluation of new OpenSourceTables

Reported by beutlich on 9 Apr 2013 14:38 UTC
According to the latest development request to implement open source tables for
Modelica.Blocks.Sources.CombiTimeTable
Modelica.Blocks.Tables.CombiTable1D
Modelica.Blocks.Tables.CombiTable1Ds
Modelica.Blocks.Tables.CombiTable2D

ITI is currently working on it. Together with the MA board we decided to provided a pre-release for early and wide evaluation.
With r6216 I committed the current project to [source:/Modelica/branches/development/OpenSourceTables OpenSourceTables].
So far I did not commit any binaries of the external library but prepared Makefiles and Visual Studio project files for self-compilation:

The MinGW-Makefile can be used for compilation and usage with OpenModelica.
The MSVC LIB Release configuration builds a static library (*.lib) with /MT and can therefore be used with Dymola.
The MSVC DLL Release configuration uses SimulationX-specific ModelicaUtilities and builds a dynamic link library (*.dll). It can therefore be used with SimulationX.

Please evaluate code and performance thoroughly! Use this ticket for your comments. Any feedback is appreciated. In order to not postpone MSL 3.2.1 release any longer rapid checks are welcome. I am afraid I cannot consider changes or wishes after 04/25/13.
TODO:

Finish the test models for the three Tables models.
More compiler tests.

This ticket addresses/fixes #129, #338, #628, #1022, #1028.

Migrated-From: https://trac.modelica.org/Modelica/ticket/1067

Comment by sjoelund.se on 10 Apr 2013 15:03 UTC
I'll start with the obvious - there are no Makefile targets for Linux and very, very poor build instructions.
MinGW works, but there are no targets for compilation against matlab/hdf5 files, and there is no zlib target for those who have it precompiled?
The 3rd-party sources used should be in the svn, or the targets should use system libraries.
Since the design seems to rely on no system libraries being used (they are not used in the Modelica annotations), these libraries should be included and the makefiles should call them in order to always go back to the same version as before. This is probably especially important since you used actual sources of libmatio anyway. Better make sure everything comes from the same version of the library...
-DHDF5 and -DMAT73 do not yield working executables unless used together, so only one should remain.
CTT.Test0 is partial but marked as an experiment. The other models are not marked partial but are not marked as experiments (you do not inherit annotations unless they say so, right?).
The problems with the tests is that they cannot be run without a working (old) Tables implementation. And once the old tables are replaced, there are no more tests to run, right (so put the old functions under NewTables.Test.OldTables)?
I would suggest structuring the tests so it is apparent which ones require old tables and which do not.
fileName = classDirectory() + "Data/test_v4.mat" - classDirectory() was never part of any Modelica standard. ModelicaServices has support for converting modelica:// URIs to filenames.
I could get a few models to run using setLinkerFlags("-lmatio -lhdf5 " + getLinkerFlags()); and the attached Makefile.
It is not quite there yet. There are very many I cannot test due to classDirectory() issues, etc. So there is probably more. Fix these first easy points and I will try again.

Modified by sjoelund.se on 10 Apr 2013 15:06 UTC

Comment by beutlich on 10 Apr 2013 15:38 UTC
Thanks for first feedback.
Yes and sorry, there is little Linux support here. I have no clue which system libraries I can assume to be installed. Do you have matio and hdf5 as system libraries? I would not have thought of openmpi. So best is to compile it all by yourself but I was to lazy to add a hdf Makefile. This is surely not my task. You may skip CCT.Test57 and CCT.Test58 for now. You should also know that OpenModelica support is not part of the implementation contract.
You are right about double preprocessor macros -DHDF5 and -DMAT73 that only work if specified together. I will consider this.
You may also remove old table implementation from CCT.Test0 to run the tests.
You are also right about the tests and comparison with old tables. I had this already in mind. Main focus today was on compatibility with old tables. I did not want to use any post-processor tools for comparison so I put both implementations in one model.
I can change the experiment annotations of the test models of course.
For now, you can manually replace classDirectory() by the table path in the files using search + replace.

Comment by beutlich on 10 Apr 2013 15:47 UTC
I forgot to ask why you link against libmatio.a? That should not be necessary and is also wrong since ModelicaMatIO.c baically is a patched libmatio tailored to Modelica environment.
I did not add hdf5, zlib and szip sources by purpose but stated the download links. I wanted to keep it clean and simple.

Comment by sjoelund.se on 10 Apr 2013 15:48 UTC
Replying to [comment:3 beutlich]:

Yes and sorry, there is little Linux support here. I have no clue which system libraries I can assume to be installed. Do you have matio and hdf5 as system libraries? I would not have thought of openmpi. So best is to compile it all by yourself but I was to lazy to add a hdf Makefile. This is surely not my task. You may skip CCT.Test57 and CCT.Test58 for now. You should also know that OpenModelica support is not part of the implementation contract.

The main problem is even if we have the libraries installed, someone else might not. And yes, there are 3 different versions of hdf5 only on Ubuntu Linux. They all require different flags to run. And the libraries have to be in the Modelica annotation.
The best way would be to include them in the .a-file like you do on Windows. All this needs is compiling the sources. Which means the sources for of the hd5 code (as well as libmatio) should be provided in the svn. If they are there I can compile the same version on all platforms and reduce the burden of maintenance.
This is the common way to do it anyway - either as svn externals or actually in the svn repository. Then you can always checkout an old version of the tables and get the same library you used at the time.
And yes, I can replace everything classDirectory() manually, but wouldn't all tool vendors need to do this?

Comment by sjoelund.se on 10 Apr 2013 15:54 UTC
Replying to [comment:4 beutlich]:

I forgot to ask why you link against libmatio.a? That should not be necessary and is also wrong since ModelicaMatIO.c baically is a patched libmatio tailored to Modelica environment.
I did not add hdf5, zlib and szip sources by purpose but stated the download links. I wanted to keep it clean and simple.

svn propedit svn:externals
Add for example: https://github.com/madler/zlib/tags/v<IP_ADDRESS> zlib
And the full source is automatically added (and can be disabled for users who prefer that). Then it is much easier to create Makefile targets to compile everything :)
As for libmatio... You are right, I might have been wrong since I possibly used the wrong defines and got a bad library for some reason. I had to add it before, but don't now.

Comment by beutlich on 10 Apr 2013 15:58 UTC
Actually we should not focus on the library dependencies. Project should run stand-alone without hdf5, zlib. Implementation contract is only about v4 MAT-files which do not require them. v6 MAT-files come for free anyway then. And v7 and v7.3 is only nice to have but certainly not the most important feature.

Comment by choeger on 10 Apr 2013 18:24 UTC
The best way would definetely be to use a good build system first. Handling dependencies with a sole makefile is tedious and error prone. I suggest autoconf or cmake. Both work on windows (albeit cmake is much easier there).

Comment by sjoelund.se on 10 Apr 2013 18:47 UTC
Christoph, I agree. Especially for Mac OSX it would be a pain to get all hdf stuff, etc working without a good build system. (Everything is always harder on Mac). I know how to build something that would sort of work on Linux (make a shared object instead of the static one built by default). But I know from experience this never works the way you would expect on OSX.
I would not say the build system is the main priority though. Getting it written in standard Modelica should be !#1. I can always donate a sane build system for MA once we start to maintain it as part of MSL. I would have to implement one for OpenModelica anyway.
I tested the library again without MAT73 or HDF5 and yes MATv4 still works, so I guess that is fine as long as it is not hard to enable the support manually. A good build system would still be nice to set CFLAGS, CPPFLAGS, LDFLAGS, enable/disable features, do feature tests, and so on.
And as a sidenote, autoconf is much nicer than cmake; everyone who writes something in cmake always requires the latest version because the old versions did not have everything they need. And of course not everyone has access to the latest cmake (for example, the latest Debian release still does not have cmake 2.8.6 which FMIL requires).

Comment by sjoelund.se on 10 Apr 2013 18:57 UTC
Also, why does everything always have to be password-protected? The ticket is not hidden, but most of our developers would not have access to test the library. My normal workflow is to just make it an svn external, setup build scripts inside OpenModelica, then ask people to try it out. Saves me the trouble of testing Windows versions.

Comment by beutlich on 10 Apr 2013 19:40 UTC
Replying to [comment:10 sjoelund.se]:

Also, why does everything always have to be password-protected? The ticket is not hidden, but most of our developers would not have access to test the library. My normal workflow is to just make it an svn external, setup build scripts inside OpenModelica, then ask people to try it out. Saves me the trouble of testing Windows versions.

I did not know at my initial commit that is a non-public repository. I could attach the zipped sources but this would not help you here. Maybe the admin can change read access rights.

Comment by sjoelund.se on 10 Apr 2013 20:13 UTC
Minor issue:
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c: In function ‘readTxtTable’:
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c:2752: warning: format ‘%u’ expects type ‘unsigned int *’, but argument 3 has type ‘size_t *’
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c:2759: warning: format ‘%u’ expects type ‘unsigned int *’, but argument 3 has type ‘size_t *’

Also tons of unused results from fread, probably in libmatio. I suppressed them.

Comment by sjoelund.se on 11 Apr 2013 07:25 UTC
Replying to [comment:9 sjoelund.se]:

And as a sidenote, autoconf is much nicer than cmake

Feel free to use the OM autoconf diffs (or the relevant part of om:source:trunk/configure.in): om:r15784 om:r15785
It seems like a reasonable way to build a dynamic library (since it is dynamic all dependencies are added). I could possibly update it a little to link zlib but not hdf5, but for just v7 MAT I didn't feel like it even though it's a small change.

Modified by beutlich on 11 Apr 2013 07:33 UTC

Comment by beutlich on 11 Apr 2013 07:46 UTC
Feel free to add/update the configuration script or Makefile.

Comment by sjoelund.se on 11 Apr 2013 09:58 UTC
I will add an autoconf target then (maybe today).
For now, this was the result of automatic testing with MSL 3.2.1 and the latest OpenModelica: http://dev.openmodelica.org/~marsj/testing/BuildModelRecursive.html
(I will update this as more commits are made and problems are fixed)

Comment by sjoelund.se on 11 Apr 2013 14:34 UTC
I added a directory using autotools under source:Modelica/branches/development/OpenSourceTables/unix/. We tested it on Linux (dynamic,zlib,hdf5), OSX (dynamic, zlib, no hdf5) and MinGW (static, no zlib or hdf5). So I am reasonably happy.
Now I'm just waiting for classDirectory() and experiment annotations.

Comment by beutlich on 11 Apr 2013 15:07 UTC
Replying to [comment:12 sjoelund.se]:

Minor issue:
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c: In function ‘readTxtTable’:
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c:2752: warning: format ‘%u’ expects type ‘unsigned int \*’, but argument 3 has type ‘size_t \*’
OpenSourceTables/Modelica/NewTables/Resources/C-Sources/ModelicaTables.c:2759: warning: format ‘%u’ expects type ‘unsigned int \*’, but argument 3 has type ‘size_t \*’

Thanks. Fixed by r6235.

Comment by sjoelund.se on 11 Apr 2013 15:31 UTC
It is fixed, but only for platforms where sizeof(long)==sizeof(size_t).
sscanf(token, "%lu", (unsigned long*)nRow) looks quite dangerous to me. You never want to sscanf into something of type size_t since the size of that type is not well-defined.

Comment by beutlich on 11 Apr 2013 15:39 UTC
Thatswhy C99 introduced %z format specifier which I cannot use for compatibility reasons.

Comment by beutlich on 11 Apr 2013 15:54 UTC
I cannot solve the other problem with classDirectory(). If replaced by standard conform funtion Modelica.Utilities.Files.loadResource() it does not work with default ModelicaServices 3.2.1.

Comment by otter on 11 Apr 2013 16:01 UTC
Replying to [comment:1 sjoelund.se]:

fileName = classDirectory() + "Data/test_v4.mat" - classDirectory() was never part of any Modelica standard. ModelicaServices has support for converting modelica:// URIs to filenames.

In the trunk of MSL there is now a function:
fileName = Modelica.Utilities.Files.loadResource(uri)
that returns the file name of an uri (such as uri = "modelica://NewTables.Test/Data/test.txt"). This function is implemented with a new ModelicaServices function "ModelicaServices.[wiki:ExternalReferences].loadResource". In Dymola this ModelicaServices function is implemented with function "Dymola_ResolveURI" that existed in previous Dymola releases. I have just committed this updated ModelicaServices for Dymola and stored it as !https://svn.modelica.org/projects/Modelica/branches/development/OpenSourceTables/Modelica/ModelicaServices-Variants/Dymola/ModelicaServices 3.2.1
Therefore, you can replace classDirectory with loadResource and use this ModelicaServices for Dymola.

Comment by sjoelund.se on 11 Apr 2013 16:02 UTC
Replying to [comment:20 beutlich]:

Thatswhy C99 introduced %z format specifier which I cannot use for compatibility reasons.

So scanf into an unsigned long which you use to assign to the size_t. Or create a utility function that scans a size_t. Inline it if you are worried about performance.
Replying to [comment:21 beutlich]:

I cannot solve the other problem with classDirectory(). If replaced by standard conform funtion Modelica.Utilities.Files.loadResource() it does not work with default ModelicaServices 3.2.1.

Then you have a problem. classDirectory() is for sure not in the specification. I would just assume the vendors have implemented loadResource in their Services.

Comment by otter on 11 Apr 2013 16:04 UTC
Replying to [comment:21 beutlich]:

I cannot solve the other problem with classDirectory(). If replaced by standard conform funtion Modelica.Utilities.Files.loadResource() it does not work with default ModelicaServices 3.2.1.

Use the just committed ModelicaServices for Dymola under
OpenSourceTables/Modelica/ModelicaServices-Variants/Dymola/ModelicaServices 3.2.1
and loadResource should be supported in Dymola

Comment by beutlich on 11 Apr 2013 19:41 UTC
Replying to [comment:19 sjoelund.se]:

It is fixed, but only for platforms where sizeof(long)==sizeof(size_t).
sscanf(token, "%lu", (unsigned long*)nRow) looks quite dangerous to me. You never want to sscanf into something of type size_t since the size of that type is not well-defined.

Yes, that was dangerous. It should be finally fixed by r6240.

Comment by sjoelund.se on 11 Apr 2013 20:47 UTC
OK, I have one more comment. The error tolerances in some of the tests (like Test14) seem almost insane to me. I had to set the dassl tolerance to 10e-12 (OM default is 10e-6) in order to get the event at exactly the same time as whatever tool generated the mat-file (Dymola?). Fixed step RK4 was fine regardless.
It makes it very hard to know if there is something wrong with a test due to table comparison not working correctly (what I assume should be tested) or if it fails due to the tolerances being too tight.

Comment by beutlich on 12 Apr 2013 07:01 UTC
Replying to [comment:26 sjoelund.se]:

OK, I have one more comment. The error tolerances in some of the tests (like Test14) seem almost insane to me. I had to set the dassl tolerance to 10e-12 (OM default is 10e-6) in order to get the event at exactly the same time as whatever tool generated the mat-file (Dymola?). Fixed step RK4 was fine regardless.
It makes it very hard to know if there is something wrong with a test due to table comparison not working correctly (what I assume should be tested) or if it fails due to the tolerances being too tight.

That is exactly the case under test. In SimulationX I can increase the solver tolerance or max. solver step size arbitrarily and the time event is always exactly met. Thus I guess there is a problem with event handling in OM.
MAT-files are generated by MATLAB R2012b.

Comment by beutlich on 12 Apr 2013 07:17 UTC
Replying to [comment:24 otter]:

Use the just committed ModelicaServices for Dymola under
OpenSourceTables/Modelica/ModelicaServices-Variants/Dymola/ModelicaServices 3.2.1
and loadResource should be supported in Dymola

By r6242 I removed calls to classDirectory() though Dymola 2013 FD01 then fails with
Compiling and linking the model (Visual C++).

Setting environment for using Microsoft Visual Studio 2008 x86 tools.
dsmodel.c
dsmodel.c(64) : warning C4013: 'Dymola_ResolveURI' undefined; assuming extern returning int
dsmodel.c(64) : warning C4047: '=' : 'const char *' differs in levels of indirection from 'int'
dsmodel.c(70) : warning C4090: 'function' : different 'const' qualifiers
dsmodel.c(436) : warning C4244: 'function' : conversion from 'double' to 'int', possible loss of data
dsmodel.c(437) : warning C4244: 'function' : conversion from 'double' to 'int', possible loss of data
   Creating library dymosim.lib and object dymosim.exp
dsmodel.obj : error LNK2019: unresolved external symbol _Dymola_ResolveURI referenced in function _Modelica_Utilities_Files_loadResource
dymosim.exe : fatal error LNK1120: 1 unresolved externals

Error generating Dymosim.

Comment by sjoelund.se on 12 Apr 2013 07:47 UTC
Replying to [comment:27 beutlich]:

In SimulationX I can increase the solver tolerance or max. solver step size arbitrarily and the time event is always exactly met. Thus I guess there is a problem with event handling in OM.

It is not really wrong. It just uses a numerical method since the trapezoid has a time event OM cannot optimize. The trapezoid expects I find time 1.5 (why the trapezoid block does not search for the exact time of the event using sampling and integer arithmetic is beyond me, but that is another matter... it will just skew a little if you have many periods).
I do find the event at time 1.500000000000001, which is the limit of double precision (searching for the state event). That is with tolerance set to 1e-12. At this time I have a slight error of 1e-15 between the table and trapezoid, which is fine (the integrated error goes to 1e-12 from 1e-31).
Tolerance set to 1e-6, I find the event at time 1.500000000000005 with an error of 5e-15, which is too high for the model (bumps the integrated error up to 7.7e-9 from 1e-31 due to the feedback, etc; the limit is set to 1e-10). But clearly dassl should accept it since it is a tiny error relative to signal...
Using tables together with models with zero-crossings is bad. If the trapezoid was modeled using time events that were easier to detect (sampling), it would be fine. Or if the tolerance hard-coded in the model wasn't set so high.
Just saying some of these models are very, very, dependent on what symbolic optimizations are performed by the tool and what kind of solver is used (rungekutta without root finding has a lower error than dassl in this test...).

Comment by sjoelund.se on 12 Apr 2013 08:19 UTC
OK, I looked closer at the implementation of the time table. It really does require you to find the exact time point for the event if you want to pass these tests. Or instead of like us the point exactly after the zero-crossing you find the point right before the zero-crossing instead.

Comment by otter on 12 Apr 2013 09:09 UTC
Replying to [comment:28 beutlich]:

Replying to [comment:24 otter]:

Use the just committed ModelicaServices for Dymola under
OpenSourceTables/Modelica/ModelicaServices-Variants/Dymola/ModelicaServices 3.2.1
and loadResource should be supported in Dymola

By r6242 I removed calls to classDirectory() though Dymola 2013 FD01 then fails with

Strange: I tried with Dymola 2013 FD01, and for me it works. From the error message it seems that the right library is loaded since function Dymola_ResolveURI is called (but maybe you double check by clicking on the Info layer of package ModelicaServices and check the file name at the bottom of the info), but linking fails. If linking fails, it might be that for some (strange) reasons the compiler links to a library from another Dymola version.

Comment by beutlich on 12 Apr 2013 09:26 UTC
Which lib/obj is Dymola_ResolveURI implemented in? I searched whole program folder for "ResolveURI" and see only these reults
C:\Programme\Dymola 2013 FD01\bin\Dymola.exe
C:\Programme\Dymola 2013 FD01\bin64\Dymola.exe
C:\Programme\Dymola 2013 FD01\Modelica\Library\ModelicaServices 1.2\package.mo

Comment by hansolsson on 12 Apr 2013 09:40 UTC
Replying to [comment:32 beutlich]:

Which lib is Dymola_ResolveURI implemented? I searched whole program folder for "ResolveURI" and see only these reults
C:\Programme\Dymola 2013 FD01\bin\Dymola.exe
C:\Programme\Dymola 2013 FD01\bin64\Dymola.exe
C:\Programme\Dymola 2013 FD01\Modelica\Library\ModelicaServices 1.2\package.mo

There are restrictions on it that makes it unsuitable for a lib (although I could not find the problematic case in the Modelica-package):
It can only resolve constant URIs; i.e. similarly as classDirectory it can handle ResolveURI("modelica://NewTables.Test/data4.mat") - but it cannot currently handle ResolveURI(a+b); unless a+b are string constants.
If it fails for a string constant it must be that the string constant is not considered as constant - I cannot see why that would happen.
If the data is stored in a data-base translation might also have to project the data to file - and then return the file-name.
(And yes, will add a better diagnostic for this.)

Comment by otter on 12 Apr 2013 10:03 UTC
Replying to [comment:32 beutlich]:

Which lib/obj is Dymola_ResolveURI implemented in? I searched whole program folder for "ResolveURI" and see only these reults
C:\Programme\Dymola 2013 FD01\bin\Dymola.exe
C:\Programme\Dymola 2013 FD01\bin64\Dymola.exe
C:\Programme\Dymola 2013 FD01\Modelica\Library\ModelicaServices 1.2\package.mo

Please, try in the command window of Dymola 2013 FD01 the following command:
Modelica.Utilities.Files.loadResource("modelica://NewTables.Test/data/test.txt");
If this works, then this indicates that loadResource is supported in this Dymola version. In case of error it shows a possible reason (e.g. because wrong Modelica package was loaded).

Comment by beutlich on 12 Apr 2013 10:57 UTC
Replying to [comment:33 hansolsson]:

If it fails for a string constant it must be that the string constant is not considered as constant - I cannot see why that would happen.

I figured it out (see e.g. changes by r6244 in NewTables.Test.CombiTable2D.Test6): If the fileName is passed directly as constructor argument to the CombiTable2D compilation fails. But using an auxiliary parameter String it works.
Now it is time to test the real thing like e.g. comment:30 started with.

Comment by beutlich on 15 Apr 2013 20:20 UTC
I got the compilation test results from dSPACE: There are no compiler warnings or errors on dSPACE targets DS1005, DS1401 and SCALEXIO. For DS1006 there are 17 warnings of the kind warning: duplicate const'` in ModelicaTables.c for C-expressions like max(max(x,y),z). These warnings can be set silent by GCC compiler option -std=gnu99.
Beside dSPACE target compilers I also successfully checked MinGW (GCC 4.4.0 and GCC 4.7.2), Cygwin (GCC 4.3.0), OpenWatCom 1.3, LCC 2.x, Borland BCC 5.5 and MS compilers (VC6 and >= VS2005 (Win32 and x64)).

Comment by beutlich on 15 Apr 2013 20:27 UTC
Replying to [comment:1 sjoelund.se]:

The problems with the tests is that they cannot be run without a working (old) Tables implementation. And once the old tables are replaced, there are no more tests to run, right (so put the old functions under NewTables.Test.OldTables)?
I would suggest structuring the tests so it is apparent which ones require old tables and which do not.

I am not satisfied with that situation, too. I used the test models as my own test suite to check compatibility with old tables implementation. What I could do now is to remove all references to old table implementation (and also the runtime assertion). This allows stand-alone simulation of the new tables (however without compatibility check) now and also after the merge.
In the implementation contract it is said that there is a reference result for every test model. I could easily commit them (using SimulationX as Modelica tool).
Please advise. Thanks.

Comment by otter on 16 Apr 2013 07:10 UTC
Replying to [comment:37 beutlich]:

Replying to [comment:1 sjoelund.se]:

The problems with the tests is that they cannot be run without a working (old) Tables implementation. And once the old tables are replaced, there are no more tests to run, right (so put the old functions under NewTables.Test.[wiki:OldTables])?
I would suggest structuring the tests so it is apparent which ones require old tables and which do not.

I am not satisfied with that situation, too. I used the test models as my own test suite to check compatibility with old tables implementation. What I could do now is to remove all references to old table implementation (and also the runtime assertion). This allows stand-alone simulation of the new tables (however without compatibility check) now and also after the merge.

Please, keep the current tests, because they are very useful for any tool vendor to quickly check its implementation with the new implementation.
What would be additionally helpful is, as you proposed, to only have tests with the table blocks from Modelica.Blocks. These tests should be stored in ModelicaTest of the

maintenance branch (!https://svn.modelica.org/projects/Modelica/branches/maintenance/3.2)
and in the trunk (https://svn.modelica.org/projects/Modelica/trunk)

As a result, the test models of the maintenance branch provide results of the vendor specific implementation of 3.2 and the test models in the trunk provide results of the new table implementation. Some tools (such as Dymola) can then make regression testing and can automatically  test the differences between the two implementations. Test models of new table features for MSL 3.2.1 should be, of course, only be stored in the trunk.

In the implementation contract it is said that there is a reference result for every test model. I could easily commit them (using SimulationX as Modelica tool).

Yes, please just store in one directory csv (ASCII) references files for SimulationX. Please, use the same format as sketched in the Call for a csv-file comparison (same format as the result csv file from the FMU compliance checker: comma separated values; first row are the variable names, first column is time with name “time”, time values must be monotonically increasing; two identical time values that follow each other signal an event instant).

Comment by beutlich on 26 Apr 2013 08:29 UTC
Thanks to all evaluators for their feedback and contribution. All comments were considered.


Fixes Issue #5 & #10

Fixes Kedyn/fusliez-notes#5 & Fixes Kedyn/fusliez-notes#10

Textarea in dark mode is now having a darker background
Winning rate text is changed to white regardless of theme
Win rate progress bar's red changed to a more muted tone #A62700
Removed padding for player icon & removed opacity if Shows Players Name is false

Damn, awesome great work... I think we should discuss a bit about #13 that way we are on the same page before merging your modifications.

@Kedyn just fixed something based on the feedback from leslie and chat. please merge it if it's good. thanks!

All cool, but the player colors on blue and purple look a bit wrong... so we going with @chilblane hot fix


KAFKA-4649: Improve test coverage GlobalStateManagerImpl

Add coverage for exception paths in initialize()

@mjsax @guozhangwang @enothereska

@guozhangwang - i've removed the redundant test. Thanks for pointing out

retest this please


Add Image Serialization Plugin

This PR need to be merged after #905 that lays the groundwork for the usage of plugin inside swift-snapshot-testing.
Overview
This PR introduces a plugin for image serialization, enhancing the flexibility and extensibility of image handling within the framework. This plugin allows for the integration of various image formats through plugins, which can be dynamically discovered and registered at runtime. The core components of this implementation include the ImageSerializationPlugin protocol and the ImageSerializer class, designed to manage and utilize these plugins.
Key Features

ImageSerializationPlugin Protocol: Defines methods for encoding and decoding images, allowing conforming classes to support various image formats. It combines the functionalities of ImageSerialization and SnapshotTestingPlugin for seamless integration with the existing plugin system.

imageFormat: A static property that specifies the format supported by the plugin.
encodeImage(_:): Encodes an image into the specified format.
decodeImage(_:): Decodes image data back into an image.

ImageSerializer Class: Manages the encoding and decoding of images using available plugins. It:

Retrieves plugins from the PluginRegistry.
Encodes and decodes images using the appropriate plugin based on the requested format.
Falls back to PNG encoding/decoding if no suitable plugin is found.

Dynamic Plugin Discovery: Plugins conforming to the ImageSerializationPlugin protocol are automatically discovered and registered using the Objective-C runtime, making it easy to extend supported formats without modifying core functionality.

Benefits

Extensibility: Easily add support for new image formats that works with all the already existing strategies through plugins without altering the core library.
Modularity: Keep the core library focused on its primary responsibilities while enabling third-party developers to extend functionality.
Dynamic Behavior: Plugins are discovered and registered automatically, reducing manual configuration and enhancing ease of use.

This PR lays the groundwork for a more modular and flexible approach to image serialization within SnapshotTesting, setting the stage for future enhancements and customizations through dedicated plugins.

I hope this better explains what I’m trying to achieve with this PR. The FilePlugin doesn’t exist yet, but it’s a potential future plugin.
graph TD
    Dev --> A(Strategies)
    Dev --> B(Strategies)
    Dev --> C(Strategies)
    Dev --> D(Strategies)

    A --> swift-snapshot-testing
    B --> swift-snapshot-testing
    C --> swift-snapshot-testing
    D --> swift-snapshot-testing

    subgraph swift-snapshot-testing
        Plugin
        Plugin --> Image
        Plugin --> File
    end

    Image --> PNG
    Image --> JXL
    Image --> HEIC
    File --> Local
    File --> SFTP
    File --> S3

    classDef green fill:#B4E1A2;
    classDef yellow fill:#FFEB99;
    classDef pink fill:#F3C1D9;
    classDef gray fill:#C4C4C4;
    classDef blue fill:#A9D6E5;

    Dev:::green
    A:::yellow
    B:::yellow
    C:::yellow
    D:::yellow
    swift-snapshot-testing:::pink
    Plugin:::gray
    Image:::gray
    File:::gray
    JXL:::blue
    PNG:::blue
    HEIC:::blue
    S3:::red
    SFTP:::red

What has changed regarding the public API of swift-snapshot-testing.
Inside SnapshotTesting target:

PluginRegistry.registerPlugin has been added to manually register a plugin(on Apple platform it works automatically).
Inside AssertSnapshot.swift imageFormat has been added to allow the customization of imageFormat and all place where we can use .image stategies have now a new optional parameter imageFormat to override the imageFormat per test if needed.

The new SnapshotTestingPlugin target:

SnapshotTestingPlugin protocol that an all plugin should use to register this could change to package level.

The new ImageSerializationPlugin target:

ImageSerializationPlugin That represent what is a ImageSerializationPlugin plugin.
ImageSerialization all plugin that want to change the way image are serialized inside swift-snapshot-testing should conform to.
ImageSerializationFormat that represent the possible format for an ImageSerializationPlugin.

That's it.

A plugin can also manage the diff algorithm. Instead of pixel comparison, we can use file hashing since generated images typically yield identical results. If the file hashes differ, we can then fall back to pixel-based diffing or even apply more advanced methods, like those below.

Algorithm
Speed
Complexity
Best Use Case

Pixel-by-Pixel
Very Fast
Simple
Exact match, quick comparisons

Mean Squared Error (MSE)
Fast
Low
Rough comparisons

Structural Similarity (SSIM)
Moderate
Moderate
Perceptual similarity

Histogram Comparison
Fast-Moderate
Moderate
Similar content with transformations

Feature-Based (SIFT, SURF)
Moderate-Slow
High
Similar objects with transformations

Perceptual Hashing (pHash)
Fast
Moderate
Near-duplicate detection

Deep Learning Models
Slow
High
Semantic similarity (same object, scene)


Implicit map keys need to be followed by map values

Error:

Ok:

The error appears only if the very last character is :
I'm not so sure it should be?
const YAML = require('yaml');
const text = require('fs').readFileSync('./doc.yaml', { encoding: 'utf8' });
const cst = YAML.parseCST(text);
const doc = new YAML.Document();

let a = doc.parse(cst[0]);

console.dir(a.errors); //

test:
  - test1: test1
    test2:

https://repl.it/@MikeWild/MammothFortunateDatamining
Yaml v 1.6.0

I'm pretty sure that this was indeed a bug in the CST parser. Specifically, it was only terminating a plain-value node at a : if the subsequent character was whitespace, but not if it the input ended there. Now it does. This changes the handling of the following minimal case:
// before
YAML.parse('a:') // 'a:'

// after
YAML.parse('a:') // { a: null }

// unchanged
YAML.parse('a:\n') // { a: null }

The specific spec construct that's being handled here is c-l-block-map-implicit-value(n) which can, when you trace all of its possible values, match the following:
c-l-block-map-implicit-value(n) ::= “:” ( s-l+block-node(n,block-out) | ( e-node s-l-comments ) )
e-node ::= e-scalar ::= /* Empty */
s-l-comments ::= ( s-b-comment | /* Start of line */ ) l-comment*
s-b-comment ::= ( s-separate-in-line c-nb-comment-text? )? b-comment
b-comment ::= b-non-content | /* End of file */

I'd missed the final step there, which allows for “:” /* End of file */ to be considered a valid implicit value.

@eemeli can you make a release ?

@notiv-nt Will do, once #122 is resolved.


Fix ipv6

It would seem that a key (which references whether ipv6 should be used to run the server) was removed from the kwargs at some point. Removing the check and subsequently not passing the value seems to have no ill effects on my local.

Apologies, this was meant to be a PR to an internal fork of this project. Please feel free to delete.


Scaffold a new controller using kubebuilder

Follow the Kubebuilder documentation to scaffold a new Kubebuilder project.
Tasks:

[ ] Scaffold a new controller project using Kubebuilder with Eventing CRD as mentioned in the proposal.
[ ] Dummy controller doing nothing.

kubebuilder init --domain kyma-project.io --repo github.com/kyma-project/eventing-manager

Checklist:

[ ] Logger init
[ ] make test-only
[ ] Linting make targets
[ ] Prow build, lint and unit-test jobs
[ ] Eventing CRD struct


Add docs for hosting LNBits on Fly.io (for free)

Fly.io is a neat hosting provider that takes a docker container and runs it for you. They have a generous free tier that is enough to run LNBits w/ sqlite. This PR is just some docs on how to do it.

Codecov Report

Merging #915 (70deb7e) into main (ae1a2ac) will decrease coverage by 10.79%.
The diff coverage is n/a.

:exclamation: Current head 70deb7e differs from pull request most recent head 7d56878. Consider uploading reports for the commit 7d56878 to get more accurate results

@@             Coverage Diff             @@
##             main     #915       +/-   ##
===========================================
- Coverage   51.84%   41.04%   -10.80%
===========================================
  Files         225      225
  Lines       12351    12351
===========================================
- Hits         6403     5070     -1333
- Misses       5948     7281     +1333

Impacted Files
Coverage Δ

lnbits/wallets/lnd_grpc_files/router_pb2.py
0.00% <0.00%> (-61.91%)
:arrow_down:

lnbits/wallets/lnd_grpc_files/lightning_pb2.py
0.00% <0.00%> (-54.65%)
:arrow_down:

lnbits/wallets/lnd_grpc_files/router_pb2_grpc.py
0.00% <0.00%> (-52.03%)
:arrow_down:

...nbits/wallets/lnd_grpc_files/lightning_pb2_grpc.py
0.00% <0.00%> (-50.48%)
:arrow_down:

lnbits/wallets/lndgrpc.py
19.11% <0.00%> (-25.74%)
:arrow_down:

lnbits/wallets/lndrest.py
17.39% <0.00%> (-20.87%)
:arrow_down:

lnbits/wallets/cln.py
21.69% <0.00%> (-13.21%)
:arrow_down:

lnbits/db.py
80.80% <0.00%> (-9.61%)
:arrow_down:

lnbits/wallets/macaroon/macaroon.py
27.53% <0.00%> (-5.80%)
:arrow_down:

lnbits/bolt11.py
78.57% <0.00%> (-2.39%)
:arrow_down:

... and 2 more

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

Clever stuff!
I had to do /home/ben/.fly/bin/flyctl

Oh right, after the Fly CLI install, it spits out some text you're supposed to copy/paste to get it into your $PATH. I'll note that in the docs.


Fix missing trailing whitespace on intl. locales

If you're not using an en-US locale, pacaur doesn't add a trailing
whitespace at the end of messages that require user input.

Thanks. Do you have any idea why this specific issue arises? I've tried to look for relevant bug reports/similar issues, but haven't found anything.

Upon further investigation, I've realized that this behavior has nothing to do with pacaur.
Before I sent this PR, I was trying to fix this issue by only editing the .po file for my language (pt.po), I went ahead and added the missing trailing whitespace, and that's where I messed up, because whenever I ran this:
$ LC_ALL=pt_PT.UTF-8 ./pacaur -S pacaur
:: Package pacaur not found in repositories, trying AUR...
:: resolvendo dependências...
:: procurando por conflitos internos...
:: pacaur-4.7.6-1 está atualizado -- reinstalando

AUR Packages  (1) pacaur-4.7.6-1

:: Continuar instalação? [S/n]n
$ _

I wasn't actually getting an updated message, because I forgot to edit the uppercase string at line 359. 🙄
TL;DR: My honest mistake. The real issue is that some translators forget to add a whitespace at the end of the msgid string. Nevertheless, I do think this PR is still valid, because now the trailing whitespace is separated from the msgid string, so translators won't have to append a whitespace anymore. It's totally up to you if you don't want to change it, of course.
Thank you and I'm sorry for the confusion!

I see. I wouldn't be against fixing that space issue, but there are quite a few other locations that have space (trailing or leading). I really don't think investing time to fix this everywhere is worth it, nor do I think the empty double quotation marks is cleaner. As such, I'll simply fix the PT translation in a quick fix.

Done in 598ef0cb4545ff181e382e7eeb9b49d79d51b250.


Remove unused variables.

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

Project coverage is 76.42%. Comparing base (0c940f8) to head (2c42d53).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4396      +/-   ##
==========================================
- Coverage   81.53%   76.42%   -5.11%
==========================================
  Files         109      109
  Lines       52793    52789       -4
==========================================
- Hits        43046    40346    -2700
- Misses       9747    12443    +2696