RFC: Unicode and and escape codes in literals

Rendered

I don't have serious use cases for it in mind for \x-encoded UTF-8 in regular string literals, but I also don't think we should disallow it. That is, if we were to design the language from scratch today, I'd argue that forbidding \x in "" just makes things inconsistent and doesn't bring much value. And since it's a backwards-compatible change to make, I think we should make that change to Rust.
As mentioned in the RFC, it also helps with macros like cstr!("\xff"). (Although I suppose we could still disallow \x escape codes entirely at the point of converting it to a literal AST token.)

This RFC talks about byte string literals, e.g. b"foo". Does it also need to discuss byte literals, e.g. b'x'?
Also, the RFC as written makes it sound like \x escapes are never allowed in regular string literals.

Allow \x… escape codes in regular string literals, as long as they are valid UTF-8. E.g. "\xf0\x9f\xa6\x80"

Only extend b"", but still don't accept \x in regular string literals ("").

But something like "\x61' is fine, the escape just must be in the range \x00-\x7f. (This isn't an issue with the RFC's intent, just a point of clarification.)

Allow \x… escape codes in regular string literals, as long as they are valid UTF-8. E.g. "\xf0\x9f\xa6\x80"

Only extend b"", but still don't accept \x in regular string literals ("").

But something like "\x61' is fine, the escape just must be in the range \x00-\x7f. (This isn't an issue with the RFC's intent, just a point of clarification.)

I think the RFC wording is precisely correct here. Today, x-escapes are only allowed if they're ASCII. This RFC expands it to allow everything that is valid utf8, which includes ASCII.

Thanks for all the feedback! I'll update the document. :)

This seems ready to FCP, apart from incorporating @scottmcm's feedback.

Consider splitting the tokenizing part of this out from the rest of the RFC. Given that lang just agreed with this rationale for what should be a tokenizing problem vs a semantic problem (rust-lang/rust#102944 (comment)), we could probably do the ""\u{D8D8}" and "\xFF" are valid tokens, but not valid values" part of this quickly as it's in-line with existing and recent precedent.

I've updated the RFC by moving the "validate later" part to the future possibilities section. I'll submit a separate RFC for that.
@joshtriplett I think this is ready to FCP. :)

I think this RFC should use the characters and strings table, because it's easy to overlook cases when dealing with prose.
Here's the status quo:
| Example | # sets* | Characters | Escapes
-- | -- | -- | -- | --
Character | 'H' | 0 | All Unicode | Quote & ASCII & Unicode
String | "hello" | 0 | All Unicode | Quote & ASCII & Unicode
Raw string | r#"hello"# | <256 | All Unicode | N/A
Byte | b'H' | 0 | All ASCII | Quote & Byte
Byte string | b"hello" | 0 | All ASCII | Quote & Byte
Raw byte string | br#"hello"# | <256 | All ASCII | N/A
Here is what the RFC is proposing, AIUI, with changes in bold, and uncertain changes with ?:
| Example | # sets* | Characters | Escapes
-- | -- | -- | -- | --
Character | 'H' | 0 | All Unicode | Quote & ASCII [1] & Unicode
String | "hello" | 0 | All Unicode | Quote & Byte & Unicode
Raw string | r#"hello"# | <256 | All Unicode | N/A
Byte | b'H' | 0 | All ASCII [2] | Quote & Byte & Unicode? [3]
Byte string | b"hello" | 0 | All Unicode | Quote & Byte & Unicode
Raw byte string | br#"hello"# | <256 | All ASCII [4] | N/A
I have numbered some inconsistencies.

[1] Should this be Byte? '¥' is already allowed. Why not '\xa5', its equivalent?
[2] Should this be All Unicode? b'\xa5' is already allowed. Why not b'¥'?
[3] (Already suggested) b\xa5 is already allowed. Why not b\u{a5}?
[4] Should this be All Unicode? If we are going to allow b"¥¥¥", why not allow br"¥¥¥"?

Answering all of those questions in the affirmative gives the maximally permissive, maximally consistent table:
| Example | # sets* | Characters | Escapes
-- | -- | -- | -- | --
Character | 'H' | 0 | All Unicode | Quote & Byte & Unicode
String | "hello" | 0 | All Unicode | Quote & Byte & Unicode
Raw string | r#"hello"# | <256 | All Unicode | N/A
Byte | b'H' | 0 | All Unicode | Quote & Byte & Unicode
Byte string | b"hello" | 0 | All Unicode | Quote & Byte & Unicode
Raw byte string | br#"hello"# | <256 | All Unicode | N/A
One possible drawback is that it becomes more important to understand that vanilla string literals are utf8 encoded while char literals are not. Which means that '\xa5' would be valid, while "\xa5" would not. But this is just a slight extension of the existing drawback of this RFC as written.

An alternative, more minimal proposal would be this:
| Example | # sets* | Characters | Escapes
-- | -- | -- | -- | --
Character | 'H' | 0 | All Unicode | Quote & ASCII & Unicode
String | "hello" | 0 | All Unicode | Quote & ASCII & Unicode
Raw string | r#"hello"# | <256 | All Unicode | N/A
Byte | b'H' | 0 | All ASCII | Quote & Byte
Byte string | b"hello" | 0 | All Unicode | Quote & Byte & Unicode
Raw byte string | br#"hello"# | <256 | All Unicode | N/A
Arguments in favour:

Still solves the primary motivation: "byte string literals are currently not a superset of regular string literals".
A near-minimal change.

The minimal change would leave raw byte string literals alone, but that seems silly. Byte string literals and raw byte string literals should behave the same except for escape handling.

All other possible changes could cause confusion, without a clear benefit.

Arguments against:

Byte literals are the odd one out. But then, they already are the odd one out. E.g. they're not a superset of char literals the way byte string literals are a superset of string literals.

Right now, this is the direction I am leaning in.

[1] Should this be Byte? '¥' is already allowed. Why not '\xa5', its equivalent?

[2] Should this be All Unicode? b'\xa5' is already allowed. Why not b'¥'?

[3] (Already suggested) b\xa5 is already allowed. Why not b\u{a5}?

[4] Should this be All Unicode? If we are going to allow b"¥¥¥", why not allow br"¥¥¥"?

@nnethercote These questions seem to be mixing up a character's codepoint with its UTF-8 representation.
\xa5 is invalid unicode. '¥' has codepoint 0xa5, which in UTF-8 is encoded as two bytes: "\xc2\xa5".
b'¥' doesn't work because that's two bytes, not one. Same for b'\u{a5}'.
b'\u{30}' should be fine though. That's just a single byte as UTF-8 (so, ascii).

I think this RFC should use the characters and strings table

My proposal is basically to just remove the "Characters" and "Escapes" column, and replace it by a requirement that some types of literals must be valid UTF-8. All literals then accept all escape codes, and validation is now about whether the result is valid UTF-8, after the escape codes have been processed.
The only open question is what to do with character literals, since multi-character literals are parsed as two lifetimes rather than as an opening and closing quote. But that seems fine to me, because allowing '\xc2\xa5' would be a bit weird anyway, considering that char doesn't store UTF-8. ('\x30' or '¥' is fine though.)

I have been assuming that UTF-8 encoding is irrelevant for char literals and byte literals, and relevant for the other four kinds of literal.
You seem to agree on this for char literals:

The only open question is what to do with character literals... allowing '\xc2\xa5' would be a bit weird anyway, considering that char doesn't store UTF-8.

What about byte literals? A u8 also doesn't store UTF-8. With that in mind, I think my questions above do make sense.

My proposal is basically to just remove the "Characters" and "Escapes" column, and replace it by a requirement that some types of literals must be valid UTF-8. All literals then accept all escape codes, and validation is now about whether the result is valid UTF-8, after the escape codes have been processed.
The only open question is what to do with character literals...

This text says "some types of literals", then "all literals", then adds an exception for char literals. Also, "All literals then accept all escape codes" is clearly imprecise because raw string literals and raw byte string literals don't accept any escape codes.
The reason I like the table approach is that it forces us to be precise and look at every possibility; it shows there are lots of different possibilities. I find it easier and more precise to think about.

I do think we should permit br"¥¥¥", but I don't think we should make any of the other changes proposed in that table, for the reasons @m-ou-se stated.
I'm going to go ahead and propose FCP for this. This does not preclude making further changes to how this information is presented.
@rfcbot merge
@rfcbot concern raw-byte-strings-with-unicode

@rfcbot fcp reviewed
This makes sense to me.

I will say this one more time: this RFC should use the characters and strings tables, as I mentioned above. That would be unambiguous. The current RFC is all text, is unclear about some cases, and then some of the comments discuss probable modifications. All of which makes the final proposal unclear.
But I don't seem to be getting much traction with this argument, so I'll just suggest that whoever implements this use the characters and strings table to drive the work. You'll have a better time that way, because you won't be able to overlook any cases.

@nnethercote I'll still update the RFC to clarify, no worries. I just haven't found the time to do that yet.

@rfcbot concern waiting-on-update-re-using-char-and-string-tables
just formally filing a concern to ensure that @nnethercote 's comment above isn't forgotten.

Revisited in lang team triage; we agreed this looks like it's waiting on the author and would wait to look again until after it was updated.

@m-ou-se ping? This is blocked waiting on updates to the RFC.

Thanks for all the feedback and patience! I finally got around to updating the RFC. I think it's all much clearer now. :)

@rfcbot resolved raw-byte-strings-with-unicode

@pnkfelix Could you please resolve waiting-on-update-re-using-char-and-string-tables?

@rfcbot resolved waiting-on-update-re-using-char-and-string-tables

Pinging @pnkfelix, @scottmcm, and @tmandry for checkboxes, now that concerns have been resolved.

Are numbers which don't represent a Unicode scalar value excluded from the definition of a Unicode escape (eg \u{DC00} or \u{FFFFFF})?
The Reference isn't currently very explicit about that (which it can get away with because at present those escapes can only appear in contexts where we promise valid utf-8). I think \u{DC00} does have a natural interpretation in a byte string.
If they're excluded, I think the "Valid unicode code point" text for validation of character literals is unnecessary (and perhaps misleading), as I think there's no way to write a character literal that would fail that validation rule.

This RFC has been merged, and we've opened a tracking issue: rust-lang/rust#116907
Thanks go out to the authors of this RFC for making Rust better by drafting it and pushing it through to acceptance.


chore make routine_load_consumer_pool_size can update using HTTP API

Proposed changes
Make routine_load_consumer_pool_size can update using HTTP API.

run buildall

run buildall


Build failure: botan on x86_64-darwin

Steps To Reproduce
Steps to reproduce the behavior:

build botan on x86_46_darwin

Build log
src/cli/sandbox.cpp:18:13: error: 'sandbox.h' file not found with <angled> include; use "quotes" instead
   #include <sandbox.h>
            ^~~~~~~~~~~
            "sandbox.h"
src/cli/sandbox.cpp:104:9: error: no member named 'sandbox_init' in the global namespace
   if(::sandbox_init(kSBXProfileNoWriteExceptTemporary, SANDBOX_NAMED, nullptr) < 0) {
      ~~^
src/cli/sandbox.cpp:104:22: error: use of undeclared identifier 'kSBXProfileNoWriteExceptTemporary'
   if(::sandbox_init(kSBXProfileNoWriteExceptTemporary, SANDBOX_NAMED, nullptr) < 0) {
                     ^
src/cli/sandbox.cpp:104:57: error: use of undeclared identifier 'SANDBOX_NAMED'
   if(::sandbox_init(kSBXProfileNoWriteExceptTemporary, SANDBOX_NAMED, nullptr) < 0) {
                                                        ^
4 errors generated.

Additional context
Add any other context about the problem here.
Notify maintainers
cc @7c6f434c @thillux
Metadata
Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.
[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.6.43, NixOS, 24.11 (Vicuna), 24.11.20240804.cb9a96f`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.5`
 - nixpkgs: `/nix/store/4cpakzyvfw1rmm9v5i3387x6jd2h1v86-source`

Add a :+1: reaction to issues you find important.

This is an exact duplicate of #333114 right?

This is an exact duplicate of #333114 right?

Indeed it is, sorry for the useless issue.

It's easy to internalise that GitHub search is broken on Nixpkgs (which is mostly true), so perfectly understandable not to find the duplicate given the volume…


Automated Package Upgrade

:tada: This PR is included in version 5.0.457 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


OTokenFactory Module

PR Overview
Edited: Structure setup related commits are moved to #11.
Otoken Factory Overview

OtokenFactory is the contract that use eip-1167 standard to clone Otoken contracts. It inherits a Spawner contract that already implemented eip-1167 with CREATE2 opcode (so we have deterministic address).
The Spawner contract is editted so need to be reviewed as well. Some relevant commits are f098951 and 955b5e1
Before calling createOtoken(), user can call getTargetOtokenAddresswith the same parameter to get the expected address the new cloned otoken contract will be deployed at.
The factory uses the hash of (underlying, strike, collateral, expiry, strikePrice, isPut) as an id to keep track of the addresses all the options.

Changes of name and symbol will affect the target token address but not the id, the factory won't allow a new otoken to be created if the id already existed.

User can call getOtoken(underlying, strike, collateral, expiry, strikePrice, isPut) to see if the desired series is deployed or not.

Todos

[x] High level test cases written out
[x] NatSpec for each function.
[x] Implementation
[x] Make test coverage 100% if not already

Open Questions
Naming:

Better naming for getTargetOtokenAddress
Better naming for internal variable _tokenAddresses

@haythem96 @aparnakr  I pumped the solidity version to 0.6.10 in this PR because the template from openzeppelin requires 0.6.2.
Also, starts from 0.6.8 the compiler check for license in each file, so I added UNLICENSED in each contract.

@antoncoding can we use packages for third-party contracts and libs for libraries ?

@antoncoding can we use packages for third-party contracts and libs for libraries ?

Good idea.

Updates:
Add error code system as @aparnakr suggested.

Add ErrorReporter.sol and return error code instead of revert in createOtoken().

Reference: Compound's ErrorReporter

Updates:
Add error code system as @aparnakr suggested.

Add ErrorReporter.sol and return error code instead of revert in createOtoken().

Reference: Compound's ErrorReporter

Hey @antoncoding can you move the error coding to a different PR? This is becoming too big

Guess we should remove the error reporter stuff for now?

Guess we should remove the error reporter stuff for now?

Already removed!

BTW, for testing, can we use https://www.chaijs.com/api/assert/ instead of 'expect' API ?

BTW, for testing, can we use https://www.chaijs.com/api/assert/ instead of 'expect' API ?

Yeah that's a good idea, the syntax is much cleaner.

Questions:

Will you have integration tests to test how this works with whitelist module?

@aparnakr we will need AddressBook ready to run integration tests.


Fix (minor) bug & Add option/directive (buildargs and Dockerfile importation)

Fix a (potential) bug with a 'tls' not present in my dict 'connection = docker.utils.lwargs_from_env()' (on my current system/os/docker version/python/...)
Add a argument from cli '--buildargs': through this option we can pass a file (YAML format) that contains build arguments for docker build (permit to manage my enterprise PROXY settings).
Add a directive 'import_dockerfile' inside DockerMake.yml: with this option, we can pass a path to a Dockerfile (by default the client search in build directory a 'Dockerfile' file). If a Dockerfile is present, dockermake.py preappend its contain before the 'build' directive.
Not big deal, but useful (i think) ^^
ps: sorry for my english :p

Thanks @yoyonel !

You're welcome, thanks you too :)


add undocumented flags to config docs

Description
The following config options appear to be undocumented in the Configuration section of our documentation:

long-query-time
log-path
max-writes-per-request
plugins.path (we should probably remove this option?)

Success criteria (What criteria will consider this ticket closeable?)
Let's get these added.

Plugins.path can be removed.  It is no longer applicable, the current plugin direction is static and there is no need for dynamic loading


Chrome and tuneText()

In Chrome 19 and Raphael 2.1.0 I experienced a random problem with svg text. First in line 4308:
fontSize = node.firstChild ? toInt(R._g.doc.defaultView.getComputedStyle(node.firstChild, E).getPropertyValue("font-size"), 10) : 10;
fontSize sometimes is NaN.
Second, line 4337: dy attribute in  sometimes equals y attribute in parent node , so that the text is vertically misplaced .

Bump; I'm somewhat of a newbie with Raphael, but is there a solution (or proposed solution) to this problem?

@hbarudin It's been ages since i've worked with Raphael but if i recall correctly I created my own, rather inelegant patch, that simply chose a more appropriate value for fontSize once the value that Rapheal was tracking went NaN. I can't remember the exact case where fontSize went bad but it had something to do with resizing the text container.
My patch went around this particular line of code (no idea what line it is now, it probably has changed in the past couple years) and watched for the change and then once it went NaN I looked for a CSS value in font-size and set fontSize to that value.
Like i said, this was inelegant and was used in a pinch. I eventually switched from using rapheal to something else for this particular project.
Best of luck.

Hi, thank your for this thread and the suggestions. I've added
if(isNaN(fontSize)) {
    fontSize = 10;
}``` after the line `fontSize = node.firstChild ? toInt(R._g.doc.defaultView.getComputedStyle(node.firstChild, E).getPropertyValue("font-size"), 10) : 10;` (6147 in Raphael 2.3.0) and this solved it.

Does this pr https://github.com/DmitryBaranovskiy/raphael/pull/1066 fix this problem? Why is it not merged?


Integration Test - Release - 2022/01/27 09:23:04

Auto create for integration test. AnotherAuthorUpdatedArticleFlow

@prmerger-dev-2 : Thanks for your contribution! The author(s) have been notified to review your proposed change.

Docs Build status updates of commit d00bc10:
:x: Validation status: errors
Please follow instructions here which may help to resolve issue.

File
Status
Preview URL
Details

:x:Error

Details

[Error: GitBranchDeletedOrForcePushed] Cannot sync git repo to specified commit because branch release-e9f882ceecc7488591b08431e9074251 has been deleted or has been force pushed

For more details, please refer to the build report.
If you see build warnings/errors with permission issues, it might be due to single sign-on (SSO) enabled on Microsoft's GitHub organizations. Please follow instructions here to re-authorize your GitHub account to Docs Build.
Note: Broken links written as relative paths are included in the above build report. For broken links written as absolute paths or external URLs, see the broken link report.
Note: Your PR may contain errors or warnings unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.
For any questions, please:Try searching the docs.microsoft.com contributor guidesPost your question in the Docs support channel


add public-good to default policy

Adds a second Authority to the default clusterimagepolicy which will allow images which have been signed by PGI Sigstore.
This essentially means that the policy would allow ANY image as long as it had been signed by EITHER the GitHub Fulcio OR the PGI Fulcio.
Either the github or sigstore PGI authorities can be selectively disabled with the trust.github and trust.sigstorePublic values in the values.yaml file.

Could this be parameterized during installation? I'm thinking what the best default values are here. The policy would still only apply to images coming from the configured organization, so adding PGI to the trust root is not a massive change in trust boundary.

Could this be parameterized during installation? I'm thinking what the best default values are here. The policy would still only apply to images coming from the configured organization, so adding PGI to the trust root is not a massive change in trust boundary.

Yeah, this is a good question. I don't think we should default to trusting both PGI and GitHub. I've done testing on an arrangement similar to this PR and there are pros/cons.
On the positive side, this is simple and supports both trust boundaries, and as @kommendorkapten said, it's not a massive change in trust boundary. This option does not require any custom configuration to support both PGI and GitHub.
On the negative side, this does introduce additional latency during verification, as both trust roots are attempted during every invocation. The PGI verifier will always try to hit Rekor during verification, which both adds latency, incurs load on PGI Rekor, and creates a privacy issue due to the Rekor request.
Additionally, if we add new trust boundaries (as has been discussed in private), we will want a way to configure them on/off.
I do think the most common use case is private repos (GitHub trust root), so I would prefer that to remain the default.
I would add Helm values to the effect of:
trust:
  github: true
  sigstore-public: false

Default
helm template --debug ./charts/trust-policies \
  --set policy.enabled=true \
  --set policy.organization=FOO

apiVersion: policy.sigstore.dev/v1alpha1
kind: ClusterImagePolicy
metadata:
  name: github-policy
spec:
  images:
  - glob: "**"
  authorities:

  - name: github
    keyless:
      trustRootRef: github
      identities:
      - issuer: https://token.actions.githubusercontent.com
        subjectRegExp: https://github.com/FOO/.*/\.github/workflows/.*
    rfc3161timestamp:
      trustRootRef: github
    signatureFormat: bundle
    attestations:
    - name: require-attestation
      predicateType: https://slsa.dev/provenance/v1

  - name: public-good
    keyless:
      identities:
      - issuer: https://token.actions.githubusercontent.com
        subjectRegExp: https://github.com/FOO/.*/\.github/workflows/.*
    ctlog:
      url: https://rekor.sigstore.dev
    signatureFormat: bundle
    attestations:
    - name: require-attestation
      predicateType: https://slsa.dev/provenance/v1

Disable GitHub
helm template --debug ./charts/trust-policies \
  --set policy.enabled=true \
  --set trust.github=false \
  --set policy.organization=FOO

apiVersion: policy.sigstore.dev/v1alpha1
kind: ClusterImagePolicy
metadata:
  name: github-policy
spec:
  images:
  - glob: "**"
  authorities:

  - name: public-good
    keyless:
      identities:
      - issuer: https://token.actions.githubusercontent.com
        subjectRegExp: https://github.com/FOO/.*/\.github/workflows/.*
    ctlog:
      url: https://rekor.sigstore.dev
    signatureFormat: bundle
    attestations:
    - name: require-attestation
      predicateType: https://slsa.dev/provenance/v1

Disable Sigstore Public Good
helm template --debug ./charts/trust-policies \
  --set policy.enabled=true \
  --set trust.sigstorePublic=false \
  --set policy.organization=FOO

apiVersion: policy.sigstore.dev/v1alpha1
kind: ClusterImagePolicy
metadata:
  name: github-policy
spec:
  images:
  - glob: "**"
  authorities:

  - name: github
    keyless:
      trustRootRef: github
      identities:
      - issuer: https://token.actions.githubusercontent.com
        subjectRegExp: https://github.com/FOO/.*/\.github/workflows/.*
    rfc3161timestamp:
      trustRootRef: github
    signatureFormat: bundle
    attestations:
    - name: require-attestation
      predicateType: https://slsa.dev/provenance/v1

Thanks @bdehamer! Now that I'm seeing the policies and examples, I'm wondering if the trust value should be nested under policy, so for example the command to disable github prod would be:
helm template --debug ./charts/trust-policies \
  --set policy.enabled=true \
  --set policy.trust.github=false \
  --set policy.organization=FOO

What do you think about that?


Adding in the footer and one image

Description

Added in the footer HTML elements
Added in the footer  CSS
Added one image for homepage ( optional )

Checklist

[ ] My code follows the style of the project.
[ ] My code has been self reviewed before to ensure that it is working, or will not cause issues with existing code.
[ ] My code has appropriate comments for documentation.

Have you tested to see if the image shows properly?

I think it'll work if you remove the spaces from the image name (and in the index file). So KSshoes.png instead of KS shoes.png
We can change it later if not, so no worries.

I think it'll work if you remove the spaces from the image name (and in the index file). So KSshoes.png instead of KS shoes.png
We can change it later if not, so no worries.

Alright i'll do that right now. Thanks :)


Implemented filter operator

3. Also, looks like the loop for pred_element in self.pred if self.pred is not None else []: is calling filter multiple times. Do pandas and spark provide an AND operator in their filter condition? If they do, is there a difference in efficiency when using that versus calling it in a loop?


Support new lines in <RichText> component

Fixes this usecase: https://github.com/Shopify/hydrogen/pull/2144#issuecomment-2141202029
It now creates a React fragment with the string and a <br> for each new line in a text node.

Thanks for sharing


Don't preserve local permissions for --extra-files

This changes our rsync invocation so that the remote copies of --extra-files always belong to root:root. I believe that's a safer default than copying local permissions in all cases. Without this change, files which should belong to root would need to locally belong to root as well. In cases where one would like other permissions on the remote system, one could chown them after copying.

I think some of flags where needed though. Now it is failing with: after_install # cat: /var/lib/secrets/key: No such file or directory

Well that's a bit embarrassing tbh, I forgot that -a also implies -rl, only tested with a single file and didn't run the tests locally. Seems to work now

Needed a rebase after the SSH connection changes on main, but conflicts are gone and tests are passing.


Support Integers as Enum Values

Support Integers as Enum Values
resolves finos/legend-studio#563

link this issue with https://github.com/finos/legend-studio/issues/563

resolves  finos/legend-studio#563

We should make the corresponding change in legend-pure as well I think.
If we don't, we end up in a situation where a user has defined a model using this syntax, but in Pure IDE Light we can't use the same syntax as it isn't recognized.
This causes a huge pain when debugging things like execution plan generation.
Ideally we should be able to take model definitions from Studio and paste them into Pure IDE Light and be able to generate execution plans in interpreted mode for queries on top of this model. Without making the corresponding change in legend-pure, we then have to modify the users model syntax to be valid in Pure IDE Light.

Can we also add tests to test the use of the enum and not just the definitions of enum with integer values?

close this pr for now

Close this ticket due to the reasons below.
Able to create an enum whose values are Integer, but this kind of enum can not be referenced. e.g. myEnum.1
When parsing the code myEnum.1, .1 will be treated as a Float since Float/Decimal has a higher priority than our customed token. Therefore, there will be an 'unexpected token' error when referring to this kind of enum.
Able to create an enum whose values are single-quoted Integer, but single quotes will be removed during the roundtrip.
Enum model::Something {'1'} . -> Enum model::Something {1}
There is a special logic to get rid of the single quote as long as it matches a regex pattern.


check invalid author

Docs Build status updates of commit 573e41e:
:warning: Validation status: warnings

File
Status
Preview URL
Details

microsoft-edge/accessibility.md
:warning:Warning
View
Details

microsoft-edge/accessibility.md

Line 0: [Warning] Cannot find user 'asasdasda' on GitHub

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


fluentbit conf: journal_host_path hardcoded in configmap

Host path is hardcoded here:
https://github.com/nickytd/kubernetes-logging-helm/blob/d85d670f148128fb7e5372e1c4be8643af9406ec/charts/templates/fluentbit/fluentbit-config.yaml#L55
Fix would be to replace it with
 Path                {{ .Values.fluentbit.journal_host_path }}
I added the fix in PR.

Thanks, the chart now uses the journal_host_path in the volume definition of the fluent-bit.


[Issue #303] handle OffsetCommitRequest retentionTime field convert properly

When KafkaConsumer send OffsetCommitRequest,
the field retentionTime is always set to -1,
and kafka server convert the retentionTime according to server config.
this patch add convert logic the same as kafka server
to avoid kop expire the wrong commit offset in
GroupMetadataManager.cleanupGroupMetadata logic.
test in KafkaRequestHandlerTest.testOffsetCommitRequestRetentionMs

Could you fix the BasicEndToEndPulsarTest.testNullValueMessages failure? Also the latest KoP has updated the pulsar version and supported continuous offset, you can rebase first.

Could you fix the BasicEndToEndPulsarTest.testNullValueMessages failure? Also the latest KoP has updated the pulsar version and supported continuous offset, you can rebase first.

@BewareMyPower this test can pass in my local env. i'll try.

@BewareMyPower this test can pass in my local env. i'll try.

@WJL3333 Got it. If some tests can pass in your local env but failed in CI, you can notify me and I'll rerun the tests since there's no script for contributors to rerun the tests.

@WJL3333 Got it. If some tests can pass in your local env but failed in CI, you can notify me and I'll rerun the tests since there's no script for contributors to rerun the tests.

@BewareMyPower hello，i look the ci result io.streamnative.pulsar.handlers.kop.BasicEndToEndPulsarTest-output.txt
there has one log that show create an non-partition topic and and partitioned topic. maybe some bug in the lookup logic.
in my local env
21:55:03.188 [pulsar-client-io-47-1:org.apache.pulsar.client.impl.PartitionedProducerImpl@148] INFO  org.apache.pulsar.client.impl.PartitionedProducerImpl - [test-produce-null-value] Created partitioned producer

in the ci failure log

10:27:25.303 [pulsar-client-io-47-1:org.apache.pulsar.client.impl.ProducerImpl@1269] INFO  org.apache.pulsar.client.impl.ProducerImpl - [test-produce-null-value] [null] Creating producer on cnx [id: 0x94cd3ae1, L:/<IP_ADDRESS>:50344 - R:localhost/<IP_ADDRESS>:15002]
10:27:25.316 [pulsar-client-io-47-1:org.apache.pulsar.client.impl.ProducerImpl@1320] INFO  org.apache.pulsar.client.impl.ProducerImpl - [test-produce-null-value] [test-0-2] Created producer on cnx [id: 0x94cd3ae1, L:/<IP_ADDRESS>:50344 - R:localhost/<IP_ADDRESS>:15002]

@BewareMyPower hello，i look the ci result io.streamnative.pulsar.handlers.kop.BasicEndToEndPulsarTest-output.txt
there has one log that show create an non-partition topic and and partitioned topic. maybe some bug in the lookup logic.
in my local env
21:55:03.188 [pulsar-client-io-47-1:org.apache.pulsar.client.impl.PartitionedProducerImpl@148] INFO  org.apache.pulsar.client.impl.PartitionedProducerImpl - [test-produce-null-value] Created partitioned producer

in the ci failure log

10:27:25.303 [pulsar-client-io-47-1:org.apache.pulsar.client.impl.ProducerImpl@1269] INFO  org.apache.pulsar.client.impl.ProducerImpl - [test-produce-null-value] [null] Creating producer on cnx [id: 0x94cd3ae1, L:/<IP_ADDRESS>:50344 - R:localhost/<IP_ADDRESS>:15002]
10:27:25.316 [pulsar-client-io-47-1:org.apache.pulsar.client.impl.ProducerImpl@1320] INFO  org.apache.pulsar.client.impl.ProducerImpl - [test-produce-null-value] [test-0-2] Created producer on cnx [id: 0x94cd3ae1, L:/<IP_ADDRESS>:50344 - R:localhost/<IP_ADDRESS>:15002]

Could you apply following diff and test again?
diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/BasicEndToEndTestBase.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/BasicEndToEndTestBase.java
index 34e95d9..6639bd3 100644
--- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/BasicEndToEndTestBase.java
+++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/BasicEndToEndTestBase.java
@@ -59,6 +59,7 @@ public class BasicEndToEndTestBase extends KopProtocolHandlerTestBase {
         super.internalSetup();
     }

+    @AfterClass
     @Override
     protected void cleanup() throws Exception {
         super.internalCleanup();
diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java
index 2aad5fa..5b772b7 100644
--- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java
+++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java
@@ -150,7 +150,7 @@ public abstract class KopProtocolHandlerTestBase {
         kafkaConfig.setAuthenticationEnabled(false);
         kafkaConfig.setAuthorizationEnabled(false);
         kafkaConfig.setAllowAutoTopicCreation(true);
-        kafkaConfig.setAllowAutoTopicCreationType("non-partitioned");
+        kafkaConfig.setAllowAutoTopicCreationType("partitioned");
         kafkaConfig.setBrokerDeleteInactiveTopicsEnabled(false);

         kafkaConfig.setKafkaMetadataTenant(tenant);

Could you apply following diff and test again?
diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/BasicEndToEndTestBase.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/BasicEndToEndTestBase.java
index 34e95d9..6639bd3 100644
--- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/BasicEndToEndTestBase.java
+++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/BasicEndToEndTestBase.java
@@ -59,6 +59,7 @@ public class BasicEndToEndTestBase extends KopProtocolHandlerTestBase {
         super.internalSetup();
     }

+    @AfterClass
     @Override
     protected void cleanup() throws Exception {
         super.internalCleanup();
diff --git a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java
index 2aad5fa..5b772b7 100644
--- a/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java
+++ b/tests/src/test/java/io/streamnative/pulsar/handlers/kop/KopProtocolHandlerTestBase.java
@@ -150,7 +150,7 @@ public abstract class KopProtocolHandlerTestBase {
         kafkaConfig.setAuthenticationEnabled(false);
         kafkaConfig.setAuthorizationEnabled(false);
         kafkaConfig.setAllowAutoTopicCreation(true);
-        kafkaConfig.setAllowAutoTopicCreationType("non-partitioned");
+        kafkaConfig.setAllowAutoTopicCreationType("partitioned");
         kafkaConfig.setBrokerDeleteInactiveTopicsEnabled(false);

         kafkaConfig.setKafkaMetadataTenant(tenant);


Export the function for rendering the default items

Export the function for rendering the default representation of the items.

Please branch out from development for your PR. The only branch that can be merged to master is development.

@agentS I created a slightly-bigger PR that includes your proposed change. Please see #82 .

Great, thank you so much! Is there any way I can buy you a coffee or make a small donation?

Wow, thanks! You can buy me a coffee via paypal.
I've released version 1.1.4. To extend the default UI, please check README examples.


platformio: 5.2.4 -> 5.2.5

Motivation for this change
Bump platformio to its most recent release.
Things done

Built on platform(s)

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

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

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

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

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module
[ ] (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes

[x] Fits CONTRIBUTING.md.

Result of nixpkgs-review pr 159034 run on x86_64-linux 1

2 packages built:

esphome
platformio

I just rebased this PR against master after #161958 landed.


Jupyter notebook examples in the docs

We apparently also get scipy 1.14.1 for free here, which fixes the Python 3.13 CI job 🙃


testing error

2023-08-22 22:36:19.798 | INFO     | main:test_lanenet:127 - Single imgae inference cost time: 0.00926s
2023-08-22 22:36:19.798 | ERROR    | lanenet_model.lanenet_postprocess:_embedding_feats_dbscan_cluster:170 - Found array with 0 sample(s)
(shape=(0, 4)) while a minimum of 1 is required by StandardScaler.
Traceback (most recent call last):
File "tools/test_lanenet.py", line 169, in
test_lanenet(args.image_path, args.weights_path, with_lane_fit=args.with_lane_fit)
File "tools/test_lanenet.py", line 139, in test_lanenet
LOG.info('Model have fitted {:d} lanes'.format(len(lane_params)))
TypeError: object of type 'NoneType' has no len()

完整的报错是：
PS E:\lanenet-lane-detection> python tools/test_lanenet.py --weights_path ./model/tusimple_lanenet/tusimple_lanenet.ckpt --image_path ./d
ata/tusimple_test_image/2.jpg
X:\Anaconda\envs\tfgpu112\lib\site-packages\tensorflow\python\framework\dtypes.py:523: FutureWarning: Passing (type, 1) or '1type' as a s
ynonym of type is deprecated; in a future version of numpy, it will be understood as (type, (1,)) / '(1,)type'.
_np_qint8 = np.dtype([("qint8", np.int8, 1)])
X:\Anaconda\envs\tfgpu112\lib\site-packages\tensorflow\python\framework\dtypes.py:524: FutureWarning: Passing (type, 1) or '1type' as a s
ynonym of type is deprecated; in a future version of numpy, it will be understood as (type, (1,)) / '(1,)type'.
_np_quint8 = np.dtype([("quint8", np.uint8, 1)])
X:\Anaconda\envs\tfgpu112\lib\site-packages\tensorflow\python\framework\dtypes.py:525: FutureWarning: Passing (type, 1) or '1type' as a s
ynonym of type is deprecated; in a future version of numpy, it will be understood as (type, (1,)) / '(1,)type'.
_np_qint16 = np.dtype([("qint16", np.int16, 1)])
X:\Anaconda\envs\tfgpu112\lib\site-packages\tensorflow\python\framework\dtypes.py:526: FutureWarning: Passing (type, 1) or '1type' as a s
ynonym of type is deprecated; in a future version of numpy, it will be understood as (type, (1,)) / '(1,)type'.
_np_quint16 = np.dtype([("quint16", np.uint16, 1)])
X:\Anaconda\envs\tfgpu112\lib\site-packages\tensorflow\python\framework\dtypes.py:527: FutureWarning: Passing (type, 1) or '1type' as a s
ynonym of type is deprecated; in a future version of numpy, it will be understood as (type, (1,)) / '(1,)type'.
_np_qint32 = np.dtype([("qint32", np.int32, 1)])
X:\Anaconda\envs\tfgpu112\lib\site-packages\tensorflow\python\framework\dtypes.py:532: FutureWarning: Passing (type, 1) or '1type' as a s
ynonym of type is deprecated; in a future version of numpy, it will be understood as (type, (1,)) / '(1,)type'.
np_resource = np.dtype([("resource", np.ubyte, 1)])
2023-08-22 22:40:02.198 | INFO     | main:test_lanenet:83 - Start reading image and preprocessing
2023-08-22 22:40:02.198 | INFO     | main:test_lanenet:89 - Image load complete, cost time: 0.00000s
2023-08-22 22:40:05.636406: I tensorflow/core/platform/cpu_feature_guard.cc:141] Your CPU supports instructions that this TensorFlow bina
ry was not compiled to use: AVX2
2023-08-22 22:40:05.758038: I tensorflow/core/common_runtime/gpu/gpu_device.cc:1432] Found device 0 with properties:
name: NVIDIA GeForce RTX 3070 Laptop GPU major: 8 minor: 6 memoryClockRate(GHz): 1.56
pciBusID: 0000:01:00.0
totalMemory: 8.00GiB freeMemory: 6.96GiB
2023-08-22 22:40:05.758341: I tensorflow/core/common_runtime/gpu/gpu_device.cc:1511] Adding visible gpu devices: 0
2023-08-22 22:40:06.325996: I tensorflow/core/common_runtime/gpu/gpu_device.cc:982] Device interconnect StreamExecutor with strength 1 ed
ge matrix:
2023-08-22 22:40:06.326164: I tensorflow/core/common_runtime/gpu/gpu_device.cc:988]      0
2023-08-22 22:40:06.326246: I tensorflow/core/common_runtime/gpu/gpu_device.cc:1001] 0:   N
2023-08-22 22:40:06.326427: I tensorflow/core/common_runtime/gpu/gpu_device.cc:1115] Created TensorFlow device (/job:localhost/replica:0/
task:0/device:GPU:0 with 7372 MB memory) -> physical GPU (device: 0, name: NVIDIA GeForce RTX 3070 Laptop GPU, pci bus id: 0000:01:00.0,
compute capability: 8.6)
2023-08-22 22:40:11.243 | INFO     | main:test_lanenet:127 - Single imgae inference cost time: 0.00919s
2023-08-22 22:40:11.258 | ERROR    | lanenet_model.lanenet_postprocess:_embedding_feats_dbscan_cluster:170 - Found array with 0 sample(s)
(shape=(0, 4)) while a minimum of 1 is required by StandardScaler.
Traceback (most recent call last):
File "tools/test_lanenet.py", line 169, in
test_lanenet(args.image_path, args.weights_path, with_lane_fit=args.with_lane_fit)
File "tools/test_lanenet.py", line 139, in test_lanenet
LOG.info('Model have fitted {:d} lanes'.format(len(lane_params)))
TypeError: object of type 'NoneType' has no len()

Hi, I met this problem as well, may I ask you how to solve this problem?

i, I met this problem as well, may I ask you how to solve this problem?


[Bug]: Terraform run time blocks up other websocket messages

The whole idea with websockets is for parallel streaming execution of stuff, but when terraform is streaming to the websocket it blocks up other requests, so there is a go routine or sometime missing in there.

@jdrodjpl Current approach: outputs from terraform stored to log file.
Reworked terraform executor to pipe output from terraform to that file.
Front-end will poll to get latest state of log file.


remove unused data from OSM files in repo

As discussed here (and following comments) there may be an interest in filtering OSM files to only include relevant data, before adding to the repo. (I'm assuming this is more relevant as an issue here than under map-matching, though I assume it applies to both.)
I definitely recommend it for new OSM files - but filtering existing files will (I think) make the repo bigger, and probably not gain us much?
Objectives for this issue:

[ ] figure out (and document) how to do the appropriate filtering
[ ] (optionally) apply it to existing OSM files

Cc @karussell @devemux86

Can you please describe a bit more what you mean by "repo"? Any why would somebody want to integrate such a functionality to graphhopper core? What is the goal (e.g. reduction of graph size,..)? From my perspective you are describing postprocessing steps one might to do and document for specific purposes. But shouldn't such steps be better kept out of scope for the graphhopper project?

Sorry @ratrun, I should've been clearer - we're not trying to add this functionality to the core (i.e. no code changes), it's just housekeeping. That is, why have full OSM files sitting in the (git) repo, when GH only needs road info, etc.? See the discussion linked above - it'd be nice to be able to add small OSM exports in as unit tests, but currently they're not small as they're full of extra junk. @karussell made the comment that it might be nice to apply this to existing OSM files in graphhoppper/map-matching.

Thanks @kodonnell, now I understand.
I agree that e.g. buildings could be filtered away, but I would not filter out any ways which potentially could be used. I would not change already checked in files as their pre-decessors would still remain in the git repro. So this idea should be only applied to new files.

I agree that e.g. buildings could be filtered away, but I would not filter out any ways which potentially could be used.

Agree. For context, the aforelinked OSM download was of only a few dozen roads, but was 6mb - in large part because of version/author information, but even things like tree species!

I would not change already checked in files as their pre-decessors would still remain in the git repro. So this idea should be only applied to new files.

There's probably a way to overwrite the history, but that scares me (and will probably cause problems) so yes, I agree.

I am adding this to this issue because it has the same reason.
I just cloned this repository to my local machine. Because I used a slow DSL connection, I wondered why it took so painfully long. The result of my researches is: This repository is blown up by a handful of of questionable binary files.
git clone git://github.com/graphhopper/graphhopper.git
Klone nach 'graphhopper' ...
remote: Counting objects: 57849, done.
remote: Compressing objects: 100% (43/43), done.
remote: Total 57849 (delta 11), reused 0 (delta 0), pack-reused 57797
Empfange Objekte: 100% (57849/57849), 79.28 MiB | 352.00 KiB/s, Fertig.
Löse Unterschiede auf: 100% (31743/31743), Fertig.

I looked which files cause the increased the huge size and have not been deleted yet using git ls-tree -r -t -l --full-name HEAD | sort -n -k 4. Here are the last lines of the output which contain the largest files:
100644 blob d4e1308cd8145b919dc5f9e078bde4b671accab7   94015	core/src/main/resources/com/graphhopper/reader/dem/Eurasia_names.txt
100644 blob adf7ddd848b2ba74168ba2df9a72134e00769f94   98361	core/files/map-sidewalk-no.osm.gz
100644 blob 5db2c6d4e9ba9bae1047b37acb13a87340206ec6  108415	web/src/main/webapp/js/lib/jquery-ui-custom-1.12.0.min.js
100644 blob 25718abea1cb6bbcdb058a2dea6040fff973779e  169124	reader-shp/src/test/resources/data/gis.osm_roads_free_1.shx
100644 blob d779934e48fafdcb7f9db465d3852540e2175a50  279487	core/files/moscow.osm.gz
100644 blob ca7200f425f376013ddafdace5ca0d399c69c48d  333976	core/files/monaco.osm.gz
100644 blob bceb49f6e4e7907a6fd11c17d251705ed354df38  449566	docs/core/images/17.jpg
100644 blob a254b23e8d6c75fd2140d9026b2c3cf48b3e5855  450118	core/files/krems.osm.gz
100644 blob 8842c67ff2817074a08c65729ceadbaa7d690abf  483425	core/files/Laufamholzstrasse.osm.xml
100644 blob 4573cae8da2240fea8091a8200de72382917507d  515085	core/files/campo-grande.osm.gz
100644 blob e9e8bc88630640e031264c543c9547c37896268f  543538	core/files/N43E007.hgt.zip
100644 blob d56e591ad7993535e2792f0d20ac191e9fee446f  565390	core/files/andorra.osm.pbf
100644 blob fb536a1de7e05bc37b1318aa742b77dfb111acd9  576904	core/files/S29W072.hgt.zip
100644 blob f14b1b59f4f7d7c92cccd68ccf736d9b9bd42515  589869	web/graphhopper-web.png
100644 blob 3d718b55708fd24c0e4793c096ff08459fb11319  594263	web/src/main/webapp/js/main.js
100644 blob 3dd4381c9dce754903ea5ddbe070a4b80be1bcbd  761456	core/src/test/resources/com/graphhopper/reader/10000EWD.txt.gz
100644 blob bbc3b2e502c930d99c0ef8b390f0624e9bccbb6f  872945	core/files/N52E008.hgt.zip
100644 blob c03811c0028de5dc7cfdce77bf0dc0747b50e670 1048676	reader-osm/files/dem140191
100644 blob c03811c0028de5dc7cfdce77bf0dc0747b50e670 1048676	reader-osm/files/dem142188
100644 blob 265f900068a565164ba7a5226a0dcb390f146384 1269768	core/files/N55W003.hgt.zip
100644 blob 5930eb29cbbd3413f7dfe33e5ea9f8621b9b7f6f 1332620	core/files/andorra.osm.gz
100644 blob 0ffd88166777fb58bc69fee039ff1b30d5407734 1358226	core/files/N49E011.hgt.zip
100644 blob 53404479bc31ba755626f039f6903ee771036467 1418473	core/files/north-bayreuth.osm.gz
100644 blob 623adc98a9e2131857aef99088068a059f69eb13 1443718	core/files/N50E011.hgt.zip
100644 blob 2c3edcd23bd2b99f2f09f576393fb3cb6fece951 2396663	web/src/main/resources/com/graphhopper/http/countries.geo.json
100644 blob 815c82541fc0547c8f33b4b71c4ec59186ebf0e8 3013882	core/files/N48W125.hgt.zip
100644 blob 491be81e73b2617753a7ee212f2f309b92cfec53 3244820	reader-shp/src/test/resources/data/gis.osm_roads_free_1.shp
100644 blob 6b84456772ce9239911cc3255d67d737f377ca25 3301079	reader-shp/src/test/resources/data/malta-latest.osm.pbf
100644 blob 116ff9c6482f0c5efe29348e1667b39033a5dc8e 3655497	reader-shp/src/test/resources/data/gis.osm_roads_free_1.dbf

One reason is commit 1960b0f which added the free road shape files of Malta by Geofabrik to the repository.
I think that these test data files should be downloaded from a third ressource (e.g. separate git repository as submodule for people really interested running unit tests of this part of the code) or should be replaced by a smaller shape file which also serves the needs or should be part of a Git submodule.
The Perl script published here also reveals other commits larger than 1000000 bytes. The SRTM tiles, the shape file and the OSM extracts are in current master branch.
bytes   SHA1
2438830 b24950147b741375eae948abaabdc7716ec55cab -- move 2.3 MB large countries.geo.json
2439610 44e954b3163e79362905fa6a2507bcb35c5518da -- rename 2.3 MB large countries.geo.json
3270496 e085c0b3f3b1e8a3b5942d0938218bec40a357f3 -- many changes in source code files, ok
2446334 0b5a60da2364abe742074c0b50f229180b060ae1 -- changes in countries.geo.json
3800201 2b888dad71178c8d767ea210a9a17e605b262205 -- new large JavaScript file
3943020 2a621f4dee90b9d78bffb601bb182730b21828fc -- in gh-script branch (rename 3.6 MB OSM PBF and 3.6 MB zipped GTFS)
3730551 7be9ec05ba3ab35ded6a7f925d3d85f380760d5c -- in fix_gh_script branch add 3.6 MB OSM PBF for the area of RNV
3738709 048df0d2b35166d76c458dc74af4fdc76d9daee1 -- addition of the same file in branch "mz" a few minutes later (maybe a rebase?)
10389816 1960b0f6239cb66fd6847fb865880a10e78eb025 -- commit discussed above introducing a shape file of Malta
3938925 06971230dd314ae38edda0cb8d8e10a6743e60af -- addition of a large JavaScript file
2151838 7d345ae02d17f2ab30b9c8cc178d00c90bf9d1bb -- addition of 3.6 MB zipped GTFS file
2939935 712787ca00d45dfd0b5f4629277d2c98220418c5 -- addition of a large JavaScript file
1578408 ea1a8e207d02cb52f2b062712b2c7e9689830c5c -- change formatting of the source code
1392049 c6c18ce28ae1722bfb6338d517ca3093e205d1ac -- change copyright notices
1408226 311969a8084a581a92cee2deb8d81899cf5dfdee -- uppercase to lowercase, camelCase to under_score
3035434 2e60c80beb19f46c14d9d3b5cccc084b873e4944 -- add a 2.9 MB SRTM tile (core/files/N48W125.hgt.zip)
2713486 0259d12b51ec924f397cdcbf4d6f2f49cec6e562 -- move three SRTM tiles, add one, delete one
1070158 5ed7bd585e5ba8d6e397e89446c7a1e30fb1c3ee -- code formatting
1548479 8c08b8c5eca24c972b5e38eccd14158fa3bc2b8f -- delete two OSM PBF files, add one (1.4 MB large)
1495218 f0e21a63d2371bb3f949cdd4e6a4a6c8c619db10 -- add a SRTM tile (core/files/N50E011hgt.zip)
3311500 0738d1ddfc8e293656e153ea676cc40a9ab199b2 -- add several SRTM tiles
1363807 68575ca5c27bd5080c3413882186490d6937c2b7 -- add Andorra as OSM PBF and gzip compressed OSM XML
1975663 5594f7f9d98d932f365557dc37b4b2d3b7abf698 -- moving many files
1170075 4104b198a88296a5bef005e4fbad660494de4f9e -- renaming packages (and therefore renaming files)
1259045 644e0d0d2dd2c9f4c3829f371ce96b1816ff664d -- moving gzip compressed OSM XML of Andorra and Monaco

Deleting them using git rm is the wrong solution because that would not reduce the size of the Git history. Instead, you have to replace the bad commits which introduced this blobs by smaller ones and rebase all newer commits on them. git filter-branch does that. There is one disadvantage: All clones of this repository have to be rebase, too, if there latest commit is newer than the oldest modified commit.

There is one disadvantage: All clones of this repository have to be rebase, too, if there latest commit is newer than the oldest modified commit.

We cannot do this :(
For new files, which procedure do you suggest? E.g. at the moment I go to osm.org, select the area that I need, download and gzip. But when doing this at a border the file is >1MB even for a very tiny area. How could I remove the relations and everything associated in this case? With some osmosis or osmium magic?

I'm using now this
osmfilter input.osm --drop="boundary=administrative" --drop-author --drop-version --drop-relations -o=output.osm

This can be done for future files only.


issue labels output is wrong with zero errors

Describe the bug
The action output for "issue-label-check" is false when zero errors are found.
Expected behavior
Outputs for a passing check should be set to true.
Screenshots
If applicable, add screenshots to help explain your problem.
Additional context
Add any other context about the problem here.

FYI @JoshuaKGoldberg - i missed this on the review. Probably pretty low impact because I don't think many people use the action outputs and the "checks" aspects are working correctly.

Err, do you have a repro link I can look at? This code looks like it should be not-buggy:
https://github.com/mtfoley/pr-compliance-action/blob/5e76f6982b6dc69630648f08556e25b4f8e93bfd/src/main.ts#L115

the checks pass fine, so that's good. !![].length outputs false it looks like, but we should be setting output to true if that check passes. ideally, the parts that make up the constant prCompliant above that are individually reflected in the outputs.
https://github.com/mtfoley/pr-compliance-action/blob/5e76f6982b6dc69630648f08556e25b4f8e93bfd/src/main.ts#L102-L107
I'll see if i can reproduce it in a public spot, but i find that I need to re-run workflows with debug logging turned on to see the output values.

Let me know if you can see this action run for reproduction:
https://github.com/mtfoley/open-sauced/actions/runs/5993189266/job/16253392863
The other outputs read true because they were successful.


BLEU metrics doc

Linked issue(s):
Fixes KOL-3433
https://linear.app/kolena/issue/KOL-3433/bleu-metrics-doc
What change does this PR introduce and why?
Adds BLEU metric doc to the metric glossary
Please check if the PR fulfills these requirements

[ x] Include reference to internal ticket and/or GitHub issue "Fixes #NNNN" (if applicable)
[ x] Relevant tests for the changes have been added
[ x] Relevant docs have been added / updated

@y27choi, can't reply to your question on whether the diagram should be included or not so I'll reply to it here. I don't think the diagram is required, but since it was a bit confusing, I think we can omit it.

Looked into using smaller sentences for the example in the doc, but the default BLEU (using 1-4 grams to calculate precision) makes the score vary a lot with small changes in the candidate sentence. For example, reference = The president spoke to the audience and candidate = The president *talked* to the audience yields a score of 0 (due to no matching 4-grams), and candidate = The president spoke to the *crowd* yields a score of 0.29 despite changing one word. Should we keep the longer example in favor of the BLEU score being more "realistic" in terms of quantifying similarity, or should we go with a shorter (6 word) sentence like above but have the score be less representative of the similarity?

Can we use a sentence that's ~ 8-10 words long? See worked example section from this article.

Finished the final review.  It looks great @billbai0102.  I just bumped one of Gordon's initial comments on the Implementation Details section.
Once that's addressed, I believe this PR is good to be merged!


How can I save video for every failed as well as passed test case?

If I want to record for each test case what should be done to store all the .mp4 files for all passed as well as failed test cases.

In the configuration object there is a parameter called saveAllVideos. Just set it to true like so:
 reporters: [
    [video, {
      saveAllVideos: true,        // If true, also saves videos for successful test cases
      videoSlowdownMultiplier: 3, // Higher to get slower videos, lower for faster videos [Value 1-100]
    }],
  ],

You can read more about the configuration options in the last three sections in the readme:
https://github.com/presidenten/wdio-video-reporter#add-the-reporter-to-config
Ps.
If you are testing on devices, make sure to update to the latest 1.2.4 release, since it adds the value of a custom capability; deviceType, as a allure argument for all test cases and also adds it to the video filenames, which makes it easier to separate desktop chrome, android phone chrome and android pad chrome for example, as well as safari for mac, iphone and ipad.
Just remember to specify it as a capability when setting up the device in appium.
filenames will be:
User-interactions--should-be-able-to-multi-select-in-dropdown-fails-by-design-to-gen-video--SAFARI--04-25-2019--17-41-54-232.mp4

User-interactions--should-be-able-to-multi-select-in-dropdown-fails-by-design-to-gen-video--SAFARI-iPad--04-25-2019--17-42-01-401.mp4

User-interactions--should-be-able-to-multi-select-in-dropdown-fails-by-design-to-gen-video--SAFARI-iPhone--04-25-2019--17-41-55-952


Improve setting a global keyboard shortcut

Closes #868
So, this rewrites our handling of accelerators a bit. First, it allows for a bit of better handling of the input (where some keys wouldn't work or show weird characters).
Second, it supports pasting a custom accelerator for edge cases like Command+Shift+5 where even though we are able to register it, we can't receive it through our input (since the system won't trigger the mouseDown event)
Finally, it adds support for accelerator custom type for plugins config.
For #868, the only workaround I was able to think of (other than manually editing the config.json file), is that you can copy the Command+Shift+5 text, and then paste in the input.

Can you describe the PR?
For example, in https://github.com/wulkano/Kap/issues/868#issuecomment-649712117 you said it was not possible. What changed?

@sindresorhus Yeah, I was still working some stuff out. Updated the description

Well, the reason I decided to support all was that I allowed for pasting as described in the description.

I don't think it's immediately obvious to the user that they can paste in the shortcut if they cannot type it in, nor what format it should be in. We need to somehow inform the user about this.

I get:

general.js:119 Uncaught TypeError: Cannot read property 'triggerCropper' of undefined

When trying to open Preferences.

I don't think it's immediately obvious to the user that they can paste in the shortcut if they cannot type it in, nor what format it should be in. We need to somehow inform the user about this.

Yeah I realized that too, but I'm not sure how to note that. It's really only for like Cmd+Shift+[1-7], so I figured if it was documented in that issue, anyone who runs into that exact problem will see the "workaround".
The preferences error is fixed 😄

Yeah I realized that too, but I'm not sure how to note that. It's really only for like Cmd+Shift+[1-7], so I figured if it was documented in that issue, anyone who runs into that exact problem will see the "workaround".

I think it will affect many, as it's common to want to use the normal screenshot keyboard shortcuts for this. Maybe we could add a small (?) type button on the edge of the input that when clicked tell the user about the pasting trick?

Could it be just a small span with ?, maybe in a circle and that has a tooltip? We don't have a popup style component

Sure

What should the copy be? Something to explain that you can paste a valid Electron accelerator string. Maybe offer the Command+Shift+5 as an example.
Thinking a different solution might be to have a little arrow there, clicking it would open a Menu with these most common shortcuts that you can't enter (Command+Shift+[1-7]), selecting one would add it as the shortcut?

I would do both. We cannot predefine all untypable keyboard shortcuts, but we can do so with the common ones.

Something to explain that you can paste a valid Electron accelerator string. Maybe offer the Command+Shift+5 as an example.

👍🏻

Updated. Discussed with @skllcrn and added the tooltip to the top-level keyboards shortcut item, so it won't be repeated if we add more later on.
Also added a dropdown with the ones I can't add. I tested again it looks like the only ones you can't enter are Cmd+Shift[3-6], so I added those

Nice work!

I noticed one thing that could be improved, but I don't think it's a new issue. If there's already a shortcut, you focus the input and then press Esc, it should just blur the input, not clear the shortcut. Another thing, if I press Shift, and then release Shift, the input field shakes, but I don't think it should when the shortcut is incomplete.


Passing tests

transformation improvements and clean up
related to https://github.com/bioimage-io/pytorch-bioimage-io/pull/7

Great Job, @FynnBe!
Regarding transformations and losses, I think an important difference is we do not track the gradient of transformations used for pre- and post- processing, but we do need that for losses. In that sense, maybe we should separate the two?
Also, a random idea, perhaps we should provide a generic transformation that automatically maps to Tensorflow/pytorch ops/layers?