Added the ability to get all Parameters on materials and also Get the Info of Parameters by index.

Thanks for the PR! I'm out traveling, so I can't really test this out until Friday, and I may have a few additional thoughts later.
Some items that stand out in my mind though:

Param vs Parameter: the existing API uses Param consistently, so Param should currently be the correct choice for name.
Perhaps a bit nitpicky, but I like to avoid the use of var in core code.
You may be mixing tabs and spaces? Alignment looks a bit off, SK's codebase uses tabs.
I need to read up on tuples or anonymous structs, or however those may be called, and figure out some details about their implementation before I take this. SK's API currently does not use that syntax feature anywhere, and relies on out parameters instead. I guess that doesn't really jive in the context of the enumerator though, and I do like the way your enumerator looks. This may also have some ramifications on the documentation tool, which isn't likely to parse this function signature correctly.

Would you prefer me to use a struct instead of a tuple
		/// <summary>
		/// The name and type of a Parameter on a Material
		/// </summary>
		public struct ParamInfo
		{
			/// <summary>
			/// The name of the param
			/// </summary>
			public string Name;

			/// <summary>
			/// The type of the param
			/// </summary>
			public MaterialParam Type;
			/// <summary>
			/// Parameter Info constructor
			/// </summary>
			/// <param name="name">Name of the shader parameter.</param>
			/// <param name="type">New type for the parameter.</param>
			internal ParamInfo(string name, MaterialParam type)
			{
				Name = name;
				Type = type;
			}
		}

Yeah, that'll also totally work :)
Also, didn't notice this before, but if you could swap this to the /develop branch, that'd be excellent! /master always points to the latest stable NuGet release. I'll merge it after that!


Fix for custom user/pwd with 'crowbarctl database'

The code was referencing the wrong variables so the default username
and password were always used. This change corrects the variable
references. Fix confirmed by sniffing the POST data and the proper
values stored in /opt/dell/crowbar_framework/config/database.yml.

 Creating crowbar database with custom user/pw failed

May need to fix the tests to take this into account, or you could change the attributes under command to be db_username/db_password instead so you probably dont need to change the tests ;)


Progress Bar v1

Jira. Step 1 of customizable progress bar
...
progress_bar: True

...
loggers:
  - progress_bar:
      dataloader_label: train

...
loggers:
  - progress_bar:
      dataloader_label: train
  - progress_bar:
      dataloader_label: eval/ep

(Added an additional day for testing with k8s formatting)
What does this mean? Is this like... running on an interactive instance and watching the bars?

What does this mean? Is this like... running on an interactive instance and watching the bars?

cc: @siriuslee can provide context here, I believe it's testing that k8s logs streaming isn't broken by this PR

What does this mean? Is this like... running on an interactive instance and watching the bars?

cc: @siriuslee can provide context here, I believe it's testing that k8s logs streaming isn't broken by this PR

Will wait for this before merging

(Let's sync offline -- sorry, there are some things I needed to update in the design doc)

Closing based on discussion


./configure --prefix support

currently --prefix seems not supported.

build system has evolved a lot. it uses CMake now. it is possible to implement --prefix feature, do we need to write document on that, @davidebeatrici  ?

Yes, we should document all the options we implement for configure.


Add "id" to App Store link.

I think you need revised URLs.  When I use this, I get "item cannot be found" in the App Store.

@nporteschaikin great catch! Thank you!

@nporteschaikin, also worth noting, you are the very first person to ever contribute to our iOS open source material :8ball:
cc @huebnerob @Dashron

@alfiehanssen :100:


remove planVersion from LogicalDataSource

Scope & Purpose
Hard-code returned "planVersion" attribute of collections to the value of 1.
Before 3.7, the most recent Plan version from the agency was returned inside "planVersion".
In 3.7, the attribute contained the Plan version that was in use when the in-memory LogicalCollection object was last constructed. The object was always reconstructed in case the underlying Plan data for the collection changed or when a collection contained links to arangosearch views.
This made the attribute relatively useless for any real-world use cases, and so we are now hard-coding it to simplify the internal code. Using the attribute in client applications is also deprecated.
Enterprise companion PR: https://github.com/arangodb/enterprise/pull/506
Docs PR: https://github.com/arangodb/docs/pull/518

[x] Strictly new functionality (i.e. a new feature / new option, no need for porting)
[x] The behavior change can be verified via automatic tests

Testing & Verification
This change is already covered by existing tests, such as all.
http://<IP_ADDRESS>:8080/view/PR/job/arangodb-matrix-pr/11023/

http://<IP_ADDRESS>:8080/view/PR/job/arangodb-matrix-pr/11371/

http://<IP_ADDRESS>:8080/view/PR/job/arangodb-matrix-pr/11381/

http://<IP_ADDRESS>:8080/view/PR/job/arangodb-matrix-pr/11384/

Tests blue


Timestamps are being escaped when used with Paperclip 4.2.1

The part right here:
https://github.com/jrgifford/delayed_paperclip/blob/master/lib/delayed_paperclip/url_generator.rb#L14-L18
Was fixed in Paperclip here: https://github.com/thoughtbot/paperclip/commit/ecfb5e2ac3257146cc3fd57ba63f52db1dff343a
But it gets monkeypatched by delayed_paperclip.

Thanks @maxim!  Any chance you could PR that over?

@ScotterC Perhaps we should get rid of alias_method_chain, instead use inheritance to write our own UrlGenerator based on paperclip's. Maybe I'll try that as a less coupled way of integrating with paperclip, let's see how that'd look.

@ScotterC oh just noticed it's a module, nevermind, will look more into it

I added an unfinished pull request, explanation there https://github.com/jrgifford/delayed_paperclip/pull/127

:+1:   Thanks @maxim

Thanks for looking into that @maxim - I'm running into the same issue and hope there'll be a working fix, soonish. Let me know if you need any help!

We'll let you know @consti!  @maxim and I spent half of this past Sunday untangling the UrlGenerator.  Hoping to have a revamped, cleaner piece of code soon

@maxim and @ScotterC, is this open issue related to why I am seeing:
imagename.jpg%3F1421493843 instead of imagename.jpg?1421493843 and my images are broken?
If it is not related I can open up a new issue.
Thank you for actively maintaining this gem! I'm looking forward to using it!

imagename.jpg%3F1421493843 instead of imagename.jpg?1421493843 and my
images are broken?

Exactly, that's the issue! :)

Until we push the fix it's best to lock into paperclip no greater than 4.2.0.

Thanks @maxim!


Update mkdirp to avoid warnings on npm install

On npm install netlify-cli the following warning is shown:
npm WARN deprecated<EMAIL_ADDRESS>Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.)

This is due to the following dependencies:
$ npm ls mkdirp
<EMAIL_ADDRESS>/home/ehmicky/.nvm/versions/node/v13.11.0/lib/node_modules/netlify-cli
├─┬<EMAIL_ADDRESS>│ └──<EMAIL_ADDRESS>deduped
├─┬<EMAIL_ADDRESS>│ └──<EMAIL_ADDRESS>deduped
└──<EMAIL_ADDRESS>
Fixing this problem would require:

[ ] Upgrading mkdirp to latest version in Netlify CLI
[ ] Upgrading mkdirp to latest version in netlify/gh-release-fetch
[ ] Switching copy-template-dir to another library, since that one is not maintained anymore

Note: make-dir is an alternative that might be better maintained.

Thanks for reporting this issue @ehmicky.

Fixed in the latest version

Currently having this issue on my computer, is there anything I can do to fix this. As I get this when I enter the command, " npm install -g express-generator"


Migrate to kubebuilder based layout

Hello!

Vote on this issue by adding a 👍 reaction
If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details
We currently use deprecated versions of the operator-framework tooling to manage this repository. We should consider migrating to 1.0.0+ supported tooling by following the migration guide here:
https://sdk.operatorframework.io/docs/building-operators/golang/migration/
This should also facilitate supporting features like #124 more easily and will make it simpler to support #123 by being more user-friendly.

This was done for v2.


[Fix #10918] Add << to Style/StringConcatenation

Closes #10918

Before submitting the PR make sure the following are checked:

[x] The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
[x] Wrote good commit messages.
[x] Commit message starts with [Fix #issue-number] (if the related issue exists).
[x] Feature branch is up-to-date with master (if not - rebase it).
[x] Squashed related commits together.
[x] Added tests.
[x] Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
[x] Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

Causes internal lints. Perhaps the cop should have this added as a config option with default false?

Hmm, I thought that + and <</concat had different semantics (new string vs modifying the receiver), but I guess this is OK for the current cop.

Causes internal lints. Perhaps the cop should have this added as a config option with default false?

It's fine to also update the existing code.

I wonder if we should also handle concat is some way, but probably that'd be an overkill.

I realized that the rule is only valid if the receiver is a string. Right now I made it check both left and right hand sides which is wrong. When I correct this, the internal lints will no longer be flagged.
[] << "foo" # not string concatenation

"#{[]}foo" # currently would yield an invalid correction

The issue mentioned this, but I missed it 🤦

It seems this cop could also detect concatenation using << in a similar way if the left-hand operand is a literal

@HeroProtagonist thanks for taking a look at this, but now I realize this substitution would be unsafe. 😞 Even if the receiver is a string literal, << cannot be replaced with interpolation if the operand is a number:
op = 65

"hello " << op    # "hello A"
"hello #{op}"     # "hello 65"

<< turns numbers into the corresponding ascii character, while interpolation turns them into decimal representation

So, what are we doing with this one?

I don't think this should be added as it only applies in a few cases. Thoughts @vlad-pisanov?

I'll take the lack of activity here as an indication we've decided to abandon this.


Clock showing full time stamp

Since the last update, %S started showing seconds and nanoseconds at the same time.

My original config:
 "clock": {
     "tooltip-format": "<tt><small>{calendar}</small></tt>",
     "format-alt": "{:%Y-%m-%d}",
     "format": "{:%Y-%m-%d %H:%M:%S}",
     "interval": 1
 },

Which did not have this issue before the update.
Could you please fix it or tell me how to change the config to fix it?

It seems to be because something changed in fmt 10.0: https://github.com/fmtlib/fmt/releases/tag/10.0.0

Implemented formatting of subseconds. For example (godbolt):

#include <fmt/chrono.h>

int main() {
  // prints 01.234567
  fmt::print("{:%S}\n", std::chrono::microseconds(1234567));
}

Thanks patrickroocks (Patrick Roocks) phprus (Vladislav Shchapov), BRevzin (Barry Revzin).

Added precision support to %S. Thanks SappyJoy (Stepan Ponomaryov)

See #2378, #2386 and please close the issue


[e2e] add support bundle test cases

Changes

ADD new test case test_secret_file_exists ref to #603
ADD new test case test_hardware_info_exists ref to #569

LGTM.
Can we take this time and handle few more things in supportbundle tests as below

The test_longfile_exists doesn't check all the log files, can we include all the log files checks?
I think we can skip the test_delete for supportbundle. It would be good if we have supportbundle after tests run.

could you make the list of all log files? the test_longfile_exists was implemented based on https://github.com/harvester/tests/issues/350, I don't have no idea how many log files is necessary and we need to verify them.
the delete method is not available for users, the test case is targeting to test the API's functionality, and we save the SB as a temporarily file, it will be removed after test finished.  and according to https://github.com/harvester/tests/issues/1037, support bundles will be clean up automatically, if we already have test report, do we still need the support bundle? (excepts upgrade testing)

I don't have no idea how many log files is necessary and we need to verify them.

If you can't add test here as you don't have idea, could you help create a test ticket to cover it.

if we already have test report, do we still need the support bundle?

Yes

I don't have no idea how many log files is necessary and we need to verify them.

If you can't add test here as you don't have idea, could you help create a test ticket to cover it.

if we already have test report, do we still need the support bundle?

Yes

so yeah, please create another ticket for your request, not ASK them in PRs.

I don't want to fight in public space, but I FREQUENTLY FEEL uncomfortable when a PR is resolving some issues, but it be asked to stack up to fix other problems.


master

update of the eq and lt method

Hi stefan,
The Comparable protocol leaves it to its subclasses to determine what equality and less than means. I don't see any benefit to adding these to the protocol and go out of line with what is in the printed book.
Thanks for reading,
David


Changing data returned by list connection

Fix #265
Description of the change
This change makes the return value of list connections to be a json containing dict of connection objects.

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Details

0 of 6 (0.0%)  changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.2%) to 50.962%

Changes Missing Coverage
Covered Lines
Changed/Added Lines
%

sdx_controller/utils/db_utils.py
0
1
0.0%

sdx_controller/controllers/connection_controller.py
0
5
0.0%

Totals

Change from base Build<PHONE_NUMBER>:
-0.2%

Covered Lines:
714

Relevant Lines:
1433

💛  - Coveralls


Table not working correctly with material design's md-checkbox

Bug, feature request, or proposal:
Bug
What is the expected behavior?
When using native checkboxes with Angular2-Data-Table, I can toggle columns on or off the table.
When trying to implement Angular2 Material Design's md-checkbox the first click doesn't work - the checkbox remains checked.
After the first click the checked value is opposite to the model, i.e. opposite to the native checkboxes.
I tried many ways to fix this but without luck. I don't know whether this is a bug in md-checkbox or in angular2-data-table.
In the plunker the code from the demo, and added md-checkboxes in addition to the native ones:
https://plnkr.co/edit/HGUjy5
What is the current behavior?
The first click should toggle the checkbox correctly.
What is the use-case or motivation for changing an existing behavior?
it would be great to use md-checkbox with the datatable
Which versions of Angular, Material, OS, browsers are affected?
latest Angular (2.1.2)
latest Angular 2 Material
latest angular2-data-table

I'm not sure what your code looks like (is there a way to view it from that plunk link?). However, I had this problem when using md-checkbox. The solution was to bind to the change event instead of the click event that you would typically use on a native checkbox.
<md-checkbox (change)="changed()"></md-checkbox>

@dsbert thank you the workaround worked !
but it's still a bug :) so leaving the issue open.
you can see the code here: https://plnkr.co/P4CeoR
just click "open in editor"

Sounds good.
side note: I just released a new version with checkbox selection built in, which might be what you were trying to do.

hi,
thanks for the info.
any idea on how I can change the select checkbox to angular material's
 ?
also, there's a bug when 'rows' attribute is initialized in the
constructor, "cant read property length of undefined" in index.js:2154
(this.rows is undefined) but i will open a separate issue on that.
On Sat, Dec 3, 2016 at 7:00 PM, Austin<EMAIL_ADDRESS>wrote:

Sounds good.
side note: I just released a new version with checkbox selection built in,
which might be what you were trying to do.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/swimlane/angular2-data-table/issues/333#issuecomment-264651244,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFyCRnV2e36miWRpYJAWbLa92tu6WRBrks5rEaAygaJpZM4LBipc
.

--
►►Rotem Meiri
<EMAIL_ADDRESS>facebook.com/rotemx

Plz open a ticket for that.
If you wanted to use material checkbox, just use it like you want any other custom control and handle selection yourself.


Allow large runners to be configured by an input

Some of the reusable workflow jobs in here refer to large runners which exist within the upbound GitHub organisation. These can't be invoked by forks:

Called workflows cannot be queued onto self-hosted runners across organisations/enterprises. Failed to queue this job. Labels: 'e2-standard-8 , linux'.

If we make this into an input then callers can customise it themselves and still continue to use the reusable workflows to publish their providers.
I have:

[x] Run make reviewable test to ensure this PR is ready for review.

How has this code been tested
I've run it here. What works is using the built-in runners like ubuntu-latest. Specifying our own runner labels doesn't work still. But at least this lets us run the reusable workflows. This inspired me to start a discussion thread on the github community.

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

Thanks @iainlane for the contribution. We have moved the upbound/provider-azuread under to the crossplane-contrib org and we can no longer run the CI jobs that depend on these larger runners. I was about to open a PR that will parameterize these runners and saw this PR.
Do you still have bandwidth to work on this PR? I will leave a review for you. If not possible, I can rebase this PR and merge it, and implement the suggested changes in a separate PR on top yours.
Thank you very much, and extremely sorry for the late reply.

I've seen the discussion you've started here:
https://github.com/orgs/community/discussions/67583
So, my understanding is that even when we parameterize the runner labels, specifying non-standard label(s) will not work, e.g., when we will be reusing this workflow and pass a parameter like:
jobs:
  ci:
    uses: upbound/uptest/.github/workflows/provider-ci.yml@main
    with:
      larger-runner: "\"Ubuntu-Jumbo-Runner\""
      ...

does not work. Is this correct?


Define Notification API on Synapsefor Images To Process

Synapse ingests everything.  DSA needs to have mechanism to know when an imaging data set is ready to be viewed in the DSA.  This means the system is at least pre-validated (i.e. file is on Synapse...)
Ideally API is such as:

Data is tagged as "Validation", "PreValidation"

API can be queries to get list of SynapseID,  S3Bucket Location, Synapse Filename, any other provenance
JSON!!!

DSA queries API, gets list of Ingestable Images

Once images are synced/imaged/indexes on the DSA, system can send back a POST request listing status of image, detected image type, metadata, etc...
System returns the DSA Identifier, DSA Path, etc, etc...

System can also return image metadata to aid the data validation pipeline... may be useful for QC

DSA API: https://imaging.htan.dev/girder/api/v1

For points 1, 2, 3, and 4 above, we can try hashing out the details in chunks. For the first chunk we can consider operating in one of these scenarios - and we can decide which scenario is most easily implementable/most relevant for HTAN (please feel free to add other scenarios if these don't make sense):

scenario 1: I have images of type X (e.g. H&E) on Synapse (living on bucket x on AWS); I have the synapse links of the images; can I send the DSA a synapse link for each image and receive a piece of metadata for it containing "is the image healthy; link to the image thumb in DSA if healthy; metadata from the imaging RFC for type X that was in the image header"; the latter request for metadata corresponds to @dgutman's point 4 above.

"I" here refers to a service the DCC runs as part of the data curator any time a data contributor indicates a dataset contains an imaging data type. In this scenario, point 1 is covered: the image links we send to the DSA are tagged as 'PreValidated'; 2 is covered, we have provided Synapse links to the DSA; 3 is covered (e.g. the DSA requests the file from Synapse using the Synapse link). How long does the POST reply for point 4 take for each image (e.g H&E; how about a more complex image type)?
The con for the scenario above is that it looks like the DSA must retrieve the image from Synapse?
scenario 2: I have images of type X (e.g. H&E) on Synapse (living on bucket x on AWS); I manage the bucket with images; can I send the DSA the bucket and receive a piece of metadata for each image containing "is the image healthy; link to the image thumb in DSA if healthy; metadata from the imaging RFC for type X that was in the image header"
"I" here refers either to a service the DCC runs as part of the data curator any time a data contributor indicates a dataset contains an imaging data type; or to a service triggered by a lambda function whenever a new image is added to the bucket. Points 1-3 are covered similarly to scenario 1 above; same question for point 4. How long does the POST reply for point 4 take for each image (e.g H&E; how about a more complex image type)?
This is similar to scenario 1, but perhaps in this case there is a way to avoid DSA 'retrieving' images via http ? (E.g. the DSA "mounts" a set of buckets predesignated for imaging)?
In either scenario, once we have the pieces of metadata corresponding to point 4, on "Synapse's" end we can validate the metadata and expose it, say, to the HTAN portal or other downstream services.
For this first chunk of work we can assume that both scenario 1 and 2 operate on public data. We can worry about using auth later once we pick a scenario?
@dgutman does it make sense to discuss this in the context of the API @xdoan linked above in a meeting? In general, we can set a small series of meetings where we cover first the questions above. Then, we can do implementation and testing on our end and have follow up questions and chunks of work (e.g. auth) covered in the next meetings?

Notes frm meeting with Dave:

He'll share a HTAN oriented set of APIs, can even customize eventually.

Things are stored as folder > item > file/files on DSA and all can have metadata

only takes a few seconds to return thumbnail/metadata

DSA can have buckets in the assetstore and technically to check header metadata/file health/get thumbnail the files only need to be touched

How to get DSA aws s3 links to imaging files uploaded through Synapse to the general bucket. Or should we just make all imaging data a custom storage location even if they use the Synapsecilent so we can easily get the image details?

possibility to mirror images in two places for storage vs performance, or to compress (will see response on RFC to compression)

APIs walkthrough here: https://github.com/ncihtan/dsa-htan/tree/master/dsaSynapseIntegration

I think this is covered by the plan here: https://github.com/sage-bionetworks/htan-coordination/issues/69


Image upload not working in IE9

As the project maintainers may have guessed (from my recent pull requests), I have the un-enviable task of getting medium-editor-insert-plugin working as close to 100% as possible in IE9.
I've gotten almost everything working now - but image uploads are still not working for me.
After applying my two most recent fixes:
https://github.com/orthes/medium-editor-insert-plugin/pull/147
https://github.com/orthes/medium-editor-insert-plugin/pull/148
Not getting any errors. However, even after including:
jquery.xdomainrequest.min.js
jquery.xdr-transport.js
jquery.iframe-transport.js
The image is still not actually getting uploaded. From IE debugbar / dev-tools network panes, and also from my server logs, I see that no POST request (or other request) is being made.
I've verified that Images.prototype.uploadAdd in medium-editor-insert-plugin is getting called (and data.submit in jquery-fileupload is getting called too); but Images.prototype.uploadDone never fires (not surprising, since there's no AJAX request happening).
Any ideas? Anything obvious that I've missed? Any more fixes that need to be made to medium-editor-insert-plugin, jquery-fileupload, or another related library? Running out of ideas here!

Thanks for your fixes so far. Since cross browser file upload might be tricky, I've decided to use blueimp's jQuery File Upload Plugin, so we can blame them not me :)
If Images.prototype.uploadAdd is called and also data.submit, the problem will be in jQuery File Upload Plugin. It looks like older IEs are not fully supported: https://groups.google.com/forum/#!searchin/jquery-fileupload/ie

For debugging purposes, I've set up a test site at:
http://mediumeditortest.vector5.com.au/
You can select an image to upload inline, and it will get sent to the server; but it doesn't get saved, the server returns back a sample image (partridge.jpg) on success.
Works fine for me on latest Chrome / Firefox / Safari, but not on IE9 - after selecting an image to upload, it just shows an empty newline in the editor content.
If anyone is able to look at this test site (has no password to access, is just prevented from being indexed in google), and to see what's going wrong, would be much appreciated.

Cross-posted at SO, in the hope that maybe with a wider audience, someone will have a solution to this:
http://stackoverflow.com/questions/29002732/medium-editor-insert-plugin-image-upload-not-working-in-ie9


solver: demanding future versions removes everything

To reproduce, start a new project with these dependencies:
    "dependencies": {
        "elm-lang/core": "2.1.0 <= v <= 2.1.0",
        "evancz/elm-html": "4.0.0 <= v <= 4.0.0"
    },

If you run elm package install, you get a fairly standard output:
$ elm package install
Some new packages are needed. Here is the upgrade plan.

  Install:
    elm-lang/core 2.1.0
    evancz/elm-html 4.0.0
    evancz/virtual-dom 2.0.0

Do you approve of this plan? (y/n) y
Downloading elm-lang/core
Downloading evancz/elm-html
Downloading evancz/virtual-dom
Packages configured successfully!

Now change the core version to a version that does not exist yet:
    "dependencies": {
        "elm-lang/core": "9.0.0 <= v <= 9.0.0",
        "evancz/elm-html": "4.0.0 <= v <= 4.0.0"
    },

Now it tries to remove everything:
$ elm package install
Some new packages are needed. Here is the upgrade plan.

  Remove:
    elm-lang/core
    evancz/elm-html
    evancz/virtual-dom

Do you approve of this plan? (y/n) y
Packages configured successfully!

...however, in practice it does not actually remove them:
$ ls -al elm-stuff/packages
total 0
drwxr-xr-x  4 rtfeldman  staff  136 Aug  4 14:43 .
drwxr-xr-x  4 rtfeldman  staff  136 Aug  4 14:42 ..
drwxr-xr-x  3 rtfeldman  staff  102 Aug  4 14:43 elm-lang
drwxr-xr-x  4 rtfeldman  staff  136 Aug  4 14:43 evancz

...but it does empty them out!
$ ls -al elm-stuff/packages/elm-lang/core/
total 0
drwxr-xr-x  2 rtfeldman  staff   68 Aug  4 14:45 .
drwxr-xr-x  3 rtfeldman  staff  102 Aug  4 14:43 ..

Expected behavior would be some sort of error explaining "there's no such version as 9.0.0" and aborting.

Ran into this myself. That was quite confusing.

Still true in 0.17.1?

Yep.

I have this issue right now. Currently, elm-package.json contains
elm-lang/navigation: "1.0.0 <= v < 2.0.0"

and when modify it to have
elm-lang/navigation: "2.0.0 <= v < 3.0.0"

then type elm-package install, it's output is:
Some new packages are needed. Here is the upgrade plan.


PHP 7.4 compatibility

This PR

adds krb5 as a dependency
sets PHP_OPENSSL=yes (workaround found in Docker)

Merged in bdeb19f83f455963fbda114bf8e217100ee2914f


Is this both STUN and TURN?

The RFC section of the README:
https://github.com/pions/turn#implemented
says that:

RFC 5389: Session Traversal Utilities for NAT (STUN)

Does this imply this can be used as a STUN server as well?
Not saying any other mention of STUN usage in the docs.

All TURN servers support STUN. See: https://www.html5rocks.com/en/tutorials/webrtc/infrastructure/

Every TURN server supports STUN: a TURN server is a STUN server with added relaying functionality built in. ICE also copes with the complexities of NAT setups: in reality, NAT 'hole punching' may require more than just a public IP:port address.

thanks


fix: Attachment - Press enter fullscreen mode in video player by Tab

Details

Fixed Issues

$ https://github.com/Expensify/App/issues/44379
PROPOSAL: https://github.com/Expensify/App/issues/44379#issuecomment-2196669019
Tests

Open the app
Go to any conversation
Click "+" and choose Add Attachment
Choose any available video
Press the Tab on the video player to fullscreen
Press enter
Verify that: Only the video player is fullscreen; the app keeps its original size
Note: It won't work on native Android because here

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

Offline tests

QA Steps

Open the app
Go to any conversation
Click "+" and choose Add Attachment
Choose any available video
Press the Tab on the video player to fullscreen
Press enter
Verify that: Only the video player is fullscreen; the app keeps its original size
Note: It won't work on native Android because here

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

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 either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[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 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(theme.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 the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[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.

Screenshots/Videos

Android: Native

https://github.com/user-attachments/assets/d375efd9-e5b9-4919-b40b-243628f6d391

Android: mWeb Chrome

https://github.com/user-attachments/assets/0a7c3dd5-9fad-4e87-a194-082178afce87

iOS: Native

https://github.com/user-attachments/assets/68cd30f6-4bb9-4ae3-af10-082039f88bb7

iOS: mWeb Safari

https://github.com/user-attachments/assets/fe9b98e3-1787-4fb0-944a-0d4ea5464908

MacOS: Chrome / Safari

https://github.com/user-attachments/assets/32d20e59-fcf6-4549-902c-d1e8779b993d

MacOS: Desktop

https://github.com/user-attachments/assets/18f88852-18f0-45e2-bba6-4afab5c6c295

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:

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

[ ] 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
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] 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).
[ ] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[ ] I verified that comments were added to code that is not self explanatory
[ ] 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.
[ ] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[ ] 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 either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[ ] 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.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] 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)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] 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
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] 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)
[ ] 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)
[ ] All JSX used for rendering exists in the render method
[ ] 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

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

[ ] 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

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

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

[ ] 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.
[ ] 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)
[ ] 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.
[ ] 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.
[ ] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[ ] I verified that all the inputs inside a form are aligned with each other.
[ ] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[ ] 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.
[ ] 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.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

https://github.com/user-attachments/assets/e2689061-44b3-491c-9f1f-b127f35a0942

iOS: mWeb Safari

https://github.com/user-attachments/assets/eca55be1-8034-4049-98b2-a24fff002273

MacOS: Chrome / Safari

https://github.com/user-attachments/assets/2d5e3c34-37ec-4587-8c5f-797591fda14d

MacOS: Desktop

https://github.com/user-attachments/assets/21fa9736-7ff0-45bb-b4b0-b0dd8834ff3c

@nkdengineer i can reproduce this on android native

friendly bump @nkdengineer

Checking this now.

@getusha Sorry for the delay
I have some problems with building android
I don't think it's a bug here
On native, when you press enter it means you press the send button

On native, when you press enter it means you press the send button

Isn't that the beahvior on every platform? when there is another element focused it shouldn't send.

I'll update today

Still investigating to find the reason

@getusha It's a weird behavior in Android simulator that we can use the tab key to focus on an element. On native, we cannot detect the focus element here, and then onPress can be triggered because shouldDisableEnterShortcut is false here.
We can't reproduce this in the real device so I don't think it's a bug.

@getusha Friendly bump.

@nkdengineer could you add a note on the PR description stating this doesn't apply for Android native?

@getusha i updated

So, is this issue/limitation only on android simulator, and not on real device? Did we confirm it?

@MonilBhavsar Yes, the we don't have the tab key in the real device. To ensure can you run an adhoc build then I can test again in the real device.

Thanks for clarifying. That's a good idea. Let's test it.

Let's merge main? I can retry then

@MonilBhavsar Merged main.

@MonilBhavsar The Android and IOS builds have failed.

Error in gemfile. Not sure why though. I have started a new build

@getusha Do you have an external keyboard to test with the real device?

@getusha ^

@getusha Do you have an external keyboard to test with the real device?

I don't have one currently, but I'll try to get it. :)

@getusha friendly bump

Any update here?

Any update here?

Lost my android phone this week, i'll update on monday 😄

Tested with external keyboard and i can reproduce this issue & i can use Tab to focus elements.
https://github.com/user-attachments/assets/74a7ca88-03b1-40a5-966c-e2b403cddf5d

you mean you can still reproduce issue on this branch? cc @nkdengineer

you mean you can still reproduce issue on this branch? cc @nkdengineer

Yes, on android native with external keyboard

I will check again.

@getusha I checked again and here is my opinion:

This bug also happens on the latest main. Let's see the video when I focus on the back button via Tab key (The button is lighter) and press enter key the attachment is also sent. So we should fix it as a separate issue to not block the current issue.

https://github.com/user-attachments/assets/3bccf3ec-c977-4423-b889-098f7d68020f

I don't feed strong that we need to fix this case since it's barely edge case since we use external keyboard for Android app.

cc @MonilBhavsar What do you think?

@getusha Any thoughts on my comment above

This bug also happens on the latest main.

@nkdengineer yes, that's what we are trying to fix.

I don't feed strong that we need to fix this case since it's barely edge case since we use external keyboard for Android app.

@MonilBhavsar what do you think? i'd say we fix this

@nkdengineer yes, that's what we are trying to fix.

@getusha I mean it's another bug. Let's see the video here, the bug doesn't happen on the web if we focus on the back button and press enter
https://github.com/user-attachments/assets/44886710-73cb-4eaa-ab07-5e3d6154860b

what do you think? i'd say we fix this

If I am understanding it correctly, the bug we're trying to fix is fixed for all platforms except android when using external keyboard, and we're thinking to leave that since it's an edge case? Isn't the root cause similar?

Isn't the root cause similar?

@MonilBhavsar Yeah the bug in Android happens when we use the tab key to focus on any button in the attachment modal not only the control video button.
And this bug doesn't happen when we focus on the back button in this screen on web like video that I shared above.

Okay, so seems like the the cause is same, but the newly reported issue while testing only happens on Android while we focus on back button? If so I think we should fix it here as they are nearly similar issues. If you're thinking other way, please let me know

Will check tomorrow

Still investigating

@MonilBhavsar After testing for many places in the App, the Android bug happens in the Modal component, other components don't face this bug. See the video here the cancel button is focused but the event of the delete button is triggered when we press on enter key. I can't find the RCA as to why this only happens in the Modal component.
https://github.com/user-attachments/assets/7dcef20e-7b11-449c-8ade-9649f62fb15a

@MonilBhavsar Any thoughts on this?

See the video here the cancel button is focused but the event of the delete button is triggered when we press on enter key.

That's the correct expected behavior, no?

Could you please describe the bug. I am not getting it, sorry.
I think if button is focussed and enter is pressed, we should trigger the event

Ah, I see. Thanks for explaining. Not easy to figure out from video 😅
Does this happen on main and for all modals? To be honest, this sounds very similar to the issue we're actually fixing

Does this happen on main and for all modals?

@MonilBhavsar Yes, it happens on the main. Currently, the bug that we're fixing in this PR is resolved except for the Android native. But it's an Android bug that happens in other places on the latest main of the App with different RCA. So I think we should ignore this here.

But it's an Android bug that happens in other places on the latest main of the App with different RCA. So I think we should ignore this here.

Okay and that does happen when using external hardware keyboard on android device. Is that right?

Okay and that does happen when using external hardware keyboard on android device. Is that right?

@MonilBhavsar Yes that happens when we use the tab key with external keyboard.

Okay cool! I think very less people would actually use external keyboard with android device and since it happens on all modals, it's fine to descope it cc @getusha

: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.


[Feature request]: Turn this into an npm package

I had thought about this.
The issue is I'm using @apply and Tailwind doesn't pickup class names from node_modules folder.
Maybe this would work when TailwindCSS v4 comes out.

I think it would be possible to add the @apply lines in the global.css using the installation script.


How to generate/set custom files for new modules

Hi,
I'm not sure how to set-up my config file (modules.php) to generate additional files. For example, I want to generate 3 different route files (frontend.php, dashboard.php and api.php).  Furthermore I want to change the generated Module ServiceProvider (add some default methods and custom traits).
Can someone tell me how to handle this? I think it could be accomplished by editing the stubs-part of the config file.
My plan is to create a new folder within my laravel-project-folder (let's name it default-module) and put all my custom module-files in there).
Thanks in advance!

Forget it, I found it out by myself 😆
I've created a subdirectory in my laravels root-directory (*/stubs/modules) and copied all content from */vendor/nwidart/laravel-modules/src/Commands/stubs to my new directory. Then I've set */config/modules.php the following way:
'stubs' => [         'enabled' => true,         'path' => base_path() . '/stubs/modules',         'files' => [             'start' => 'start.php',             'routes' => 'routes/frontend.php',             'routes_dashboard' => 'routes/dashboard.php',             'views/index' => 'Resources/views/index.blade.php',             'views/master' => 'Resources/views/layouts/master.blade.php',             'scaffold/config' => 'Config/config.php',             'composer' => 'composer.json',         ],         // other stuff     ],
As you can see, I added a new file (routes_dashboard.stub) to my stubs-directory. Now I can reference this file in the config-file and it will be created during module-creation.
That's all I was looking for! Now I can modify my mdoule-files the way I need.
Suggest: Maybe this should be described as example somewhere in the documentation...


Update the text below the ace component with the current text

To show how one can listen to updates in the ace component

Thanks! :+1:


Add task_exec and task role policy attachments

what

Creates two variables to accept a list of policies to attach to the generated task and task execution roles
The policies are only attached if the the roles were created by this module.

why

When creating services that reference outside secrets it would be easier to pass the required policy during service creation.

references

This is related to #67 and #68

@sodre the title starts with WIP. Ready to review?

@joe-niland, this should be read for review.

/test all

/test all


[WIP] vtune: init at 2022.1.0-98

Motivation for this change
WIP because vtune-gui crashes with a SIGTRAP inside libcef, but starting a recording with the vtune CLI seems to work fine.
The vtune-gui crash seems to be from a debug assertion in CEF which is embedded into the executable, and it's annoying to track down the cause without symbols.
Things done

Built on platform(s)

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

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

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

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
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.

any update?


Node module repo declared HTML

Node-RED module (based on NodeJS) labelled as HTML project. Pretty safe to say that if a project contains a package.json file and subsequent dependencies section within it that it's going to be a JavaScript based project. This issue appears on, despite placing a .gitattributes file (as suggested in prior support) as well as .getattributes'less repos:
https://github.com/Steveorevo/node-red-contrib-string
https://github.com/Steveorevo/node-red-contrib-nbrowser

Both repo's are being identified 100% correctly, regardless of your .gitattributes overrides (which aren't doing anything Linguist isn't already doing).  You're missing one vital point about how Linguist calculates the language stats (taken from the README - emphasis mine):

The Language stats bar displays languages percentages for the files in the repository. The percentages are calculated based on the bytes of code for each language as reported by the List Languages API.

For https://github.com/Steveorevo/node-red-contrib-string, you have 32.1 KB of HTML vs only 3.62 KB of Javascript, thanks to this one file (clicking the language bar and then HTML brings you to this page).
For https://github.com/Steveorevo/node-red-contrib-nbrowser, you have 40.5 KB of HTML vs 20.5 KB of Javascript thanks to these two files.
If you want your repo to appear as if it is predominantly Javascript, you'll either need to write more Javascript 😉  or mark the HTML files as vendored, generated or documentation using an override.

Ahh, explains why many node-red objects are showing up as HTML vs JavaScript. Do any of the markings for vendored, generated, or documentation preserve visibility in diffs? (rumor is vendored omits this). Thanks for the reply.

Only generated affects diff visibility (also stated in the README) so vendored should probably do the trick. Or you could add a topic to the repo as the language bar doesn't really depict the repo language; it only shows the breakdown of the languages in a repo, and HTML is a valid and common occurrence (expected, maybe?) language for Node-red repos AFAICS.

Ahh, then this open issue may no longer be an issue https://github.com/github/linguist/issues/2206
Node-RED's focus is JavaScript. It's a "runtime/design time" paradigm kinda animal. I'll try documentation first; as the HTML in Node-RED projects contain a brief "design time" explanation of the JavaScript object, which I guess is a documentation of sorts. But it also contains mostly JavaScript to aid the IDE in interfacing with said JavaScript "runtime" object.

Ahh, then this open issue may no longer be an issue #2206

Yes and I think we can probably close it as you can now mark files as generated or not in .gitattributes. We did try implementing suppression for vendored files but this was reversed but I can't remember why right now.


475 ic skeleton an example of using the skeleton in a card

Summary of the changes
Example of ic-skeleton being used within ic-card has been added to the skeleton code examples
Related issue
#475
Checklist

[ ] I have manually accessibility tested any changes, if relevant.
[ ] I have raised this pull request against the develop branch

Looks great but I'm wondering if the timeout should start when the page first loads as well as after you've clicked the refresh button? At the moment it's a bit like you're refreshing it before it's even loaded. But not a huge issue I guess


SQFormMultiSelect styling differences with other components

It looks like the styling is not being bundled for the multi select component.
Edit:
After looking into the component it looks like there is not any styles being applied for font size.  I will wait to here back from Nemeth on what should be done.
https://github.com/SelectQuoteLabs/SQForm/pull/14/files - PR to add the component.

When hovering over the selected items we also get a tooltip with the Keys.  I don't think that is probably desirable.


consider exposing size for cache entries

(I think this is a known feature request, but didn't see an issue for it.)
Consider how you would build a media application like a music player or photo gallery.  While you probably couldn't store all media offline, it would be nice to save some amount of the most frequently used files.
Currently the Cache API lets us build an LRU cache based on count:
var cache;
cache.open('foo').then(function(foo) {
  cache = foo;
  return cache.match(request);
}).then(function(response) {
  if (response) {
    // update order of entries in keys()
    cache.put(request, response.clone());
    return response;
  }

  var maxItems = 100;
  return addToLRU(cache, maxItems, request);
});

function addToLRU(cache, maxItems, request) {
  return cache.keys().then(function(keys) {
    if (keys.length < maxItems) {
      return cache.add(request);
    }

    return cache.delete(keys[0]).then(function() {
      return cache.add(request);
    });
  });
}

It would be nice, however, to be able to store items up until a certain size limit is reached.  I was thinking an API like:
  Promise<unsigned long> sizeOf(Request or USVString request, optional QueryParams params);
  Promise<unsigned long> sizeOfAll(Request or USVString request, optional QueryParams params);

These would function like match() and matchAll(), but return a size value instead of the responses.
It would then be necessary to define what "size" means.  This I am less sure of:

Size of on-disk or in-memory?  I assume on-disk would be preferable.
Size of just the body or including all fields?  The body will typically dominate for most Responses.
Require approximate or exact measurements from the browser?  Given compression, databases, and de-duplication, I think approximate would be better.

Thoughts?

Twitter asked for this too. We should have some kind of method on the cache object that provides an object of meta information, which includes size on disk.

(Since we've heard similar requests in Quota API in the past I filed a related issue, we probably have to agree on where these APIs should live on: https://github.com/kinu/quota-api/issues/10)

One more datapoint: this came up as a question on StackOverflow.

tl;dr I'd like to see the ability to query not only the size of individual items in the cache, but also the total size of the cache itself. Hopefully this can be done in a performant way, without having to spin through all the items adding up the size. Each cache could keep a tally on the total size of the items it contains and update it as items are added and removed.
Imagine a use case where you might want to take an app offline that deals with displaying large documents. You also want to enable the user to take an entire document offline. The document is comprised of multiple requests, say 1 upfront and 1 per page. When the user clicks to take a document offline, let's say we create a service worker cache for that document, request all the assets and add them to that cache.
Now the user has taken a few documents offline. There is a cache per doc, and one for the app assets. The user may want to see what documents are available offline, how much space each uses on their device, and remove a document from the offline cache. If each cache knew the size of the items it contained, then segmenting data into caches in this fashion makes it easy to query the size of a single document.
In a use case like this, the user would want to see the space actually consumed on disk, including request overhead and taking into account compression, etc.

@robrbecker your tl;dr is longer than the OP! But the extra detail is really valuable, thanks for the use-case and clarification.

the user would want to see the space actually consumed on disk, including request overhead and taking into account compression, etc

The UA may dedupe across caches to save disk space. I don't think this is a huge issue, but we should nod to it in the spec.

not only the size of individual items in the cache

In fact, the cache size is going to be more accurate than the content-size header which may be absent or completely false. An accurate way would be response.arrayBuffer().then(a => a.length), but that would be pretty horrific for performance.
@robrbecker what's more important, the size of individual items or the size of the whole cache?
@wanderview would it be simpler to have a single cache.size() promise-returning method? If we need a way to query the size of multiple cache entries maybe we should change the resolve-type of .matchAll to be something that extends Array, so it would be cache.matchAll(request).then(rs => rs.totalSize()) or something.

While you probably couldn't store all media offline, it would be nice to save some amount of the most frequently used files.

Why not store all media offline?
Similar to @robrbecker, specifics on amount of space used.. and more importantly: amount available is very important to our use case: caching large binary assets (video, images, audio) for physically installed media (digital signage,kiosks, interactive video walls, etc.) that load via web.. but may have intermittent connection.
Specs. regarding clearing of data from cache also critical. Need to understand exactly how much, and how long data will be cached before we can fully rely on it (100% offline-enabled app).

Why not store all media offline?

My 2 TB dropbox account will probably not fit on my mobile device for some years... That was the case I was referring to.

Similar to @robrbecker, specifics on amount of space used.. and more importantly: amount available is very important to our use case: caching large binary assets (video, images, audio) for physically installed media (digital signage,kiosks, interactive video walls, etc.) that load via web.. but may have intermittent connection.
Specs. regarding clearing of data from cache also critical. Need to understand exactly how much, and how long data will be cached before we can fully rely on it (100% offline-enabled app).

Yes, these cases are important too.  I believe the current leading proposal for handling guaranteed persistent storage is in here:
https://wiki.whatwg.org/wiki/Storage
It sounds like the v1 bits there would help with your use case.

I'm keen on looking at this stuff for v2. We need to make sure that by exposing size we don't hint at content of opaque responses.

@jakearchibald Finally rounding back on this issue... I think total size of a cache is more important than individual size. That may be a way to get around security concerns of giving out the exact size of each response. (Except in the degenerate case of 1 to 1 cache <-> response)

That's what I'm worried about. Actively thinking about this.

This same problem exists for exposing size estimates even on the origin with storage spec.  I guess we could just exclude opaque bodies in the script exposed size estimates, but that is kind of annoying to implement and reduces the utility here.

The total cache size of opaque items could be ceiled up to closest e.g. 100k bytes so it would be useful for total size use case but not hinting too much of the actual size of an opaque response in the 1 to 1 cache <-> response case.


Support encryption key in configuration

Support encryption key used to encrypt the entire [Realm].
Its length is forced to be 64bytes.
If null encryption is not enabled.
Fixes #88

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 4 (100.0%)  changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+5.5%) to 93.128%

Totals

Change from base Build<PHONE_NUMBER>:
5.5%

Covered Lines:
393

Relevant Lines:
422

💛  - Coveralls

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

4 of 4 (100.0%)  changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+5.5%) to 93.128%

Totals

Change from base Build<PHONE_NUMBER>:
5.5%

Covered Lines:
393

Relevant Lines:
422

💛  - Coveralls

I am writing to request clarification on the encryption key when passed, when added to realm later will it encrypt the existing realm?

also the example from docs shows encrypting local realm
// Generate encryption key
--
final key = List<int>.generate(64, (i) => Random().nextInt(256));

final encryptedConfig = Configuration.local([Car.schema],
// Include the encryption key in the configuration
encryptionKey: key);
final encryptedRealm = Realm(encryptedConfig);

does it work on flexible sync as well @desistefanova

Yes, it works with flexible sync and no - you cannot change the encryption of a file after it was created. You can use Realm.writeCopy to create an encrypted copy of the file, but that only works for local files. For synchronized realms, you can just recreate the file and re-fetch the data.


Prebuilt ZBM for EFI possibly screws up USB keyboard

Dear all,
a few days ago I installed the prebuilt ZBM EFI binary (https://github.com/zbm-dev/zfsbootmenu/releases/download/v2.2.1/zfsbootmenu-release-x86_64-v2.2.1-vmlinuz.EFI) on two of my servers. ZBM works flawlessly there with the following exception:
When ZBM starts the Linux system (Debian bullseye), the first thing the booting Debian system spits out is something like "Tearing down USB controller". When the Debian system has booted, it operates normally except that the USB keyboard is completely dead. I do not know whether it's only the keyboard or whether it's the USB controllers on the mainboard that become defunctional; I guess it's the latter, given the error message.
I do not know whether any previous version of the prebuilt ZBM EFI binary has worked correctly, because until now I generated the ZBM binaries myself. This is currently not possible; therefore I have switched to the prebuilt binaries.
To avoid misunderstandings:
When the servers in question start, they can be normally operated via USB keyboard during the various stages of the boot process, until the ZBM boot menu appears. The USB keyboard also works normally in the ZBM menu itself. However, the Linux kernel that is started by ZBM obviously cannot use the USB controller any more, or at least not the keyboard.
I didn't have that problem with the previous versions of ZBM that I had compiled and generated myself.
Best regards, and thank you very much,
Binarus

When you say server, does this machine have an IPMI/iDRAC controller? If so, then this is mostly a matter of conflicting priorities.
A number of XHCI chipsets do not properly reinitialize themselves after a kexec, so the prebuilt images automatically unbind the driver from them just prior to kexec.
A smaller number of systems have the opposite behavior, where the USB interface becomes unusable after this is done.
We've prioritized supporting the former use-case in our prebuilt releases. Upcoming binary releases will have an easier toggle to allow these types of actions to be disabled easily.  There is a way to do that now, but it's a little more involved.
I'd recommend trying to use the known-good uefi stub linked in the other issue to get a working local build. If you still have trouble with that, we can provide a bit of help to mask the XHCI teardown hook.

Thank you very much for your help!
You are absolutely right. The two servers in question are based on Supermicro X10 platforms that contain IPMI controllers. Thank you very much for your explanation. It might be a good idea to look for a BIOS update then.
I have just tried to get a working local build, but still with the stub that is included in Debian. But this time, the stub is newer, because I am now on bookworm. That didn't change anything. Still a boot prompt with some shell, showing dracut complaining about a missing or empty rootfs= variable. My gut feeling is that this is not due a broken EFI stub.
However, I'll try once again with the external known-good EFI stub you linked in the other issue and report back.
Best regards,
Binarus

OK, I just tried again.
I re-compiled zfsbootmenu 2.2.1 from scratch, downloaded the known-good EFI stub, added the path to it in config.yaml, mounted the EFI partition to /boot/efi and ran generate-zbm. This led exactly to the same situation as before.
I am now convinced that this is due to a dracut problem, although I didn't touch the dracut configuration in any way (compared to that that worked correctly with generate-zbm some time ago) and although generate-zbm -d didn't show errors that could cause the problem (IMHO).
Could you please give a short outline of the procedure we had to follow if we would like to solve the USB problem? If it involves containers, it would be beyond my horizon. I have experience with VMs and the like, but I have no clue of containers (because I didn't like the underlying concept from the beginning on :-)).
Thank you very much, and best regards,
Binarus

If you're already creating your own images, just follow these instructions to use a known-good stub loader.
If you want to use a prebuilt image, set zbm.hookroot and mask the xhci-teardown.sh script.

At first, thanks you very much!

If you're already creating your own images, just follow these instructions to use a known-good stub loader.

Well, that's what I have described one post above yours :-) It didn't work even with the known-good stub. When booting, I ran into a dracut shell where dracut complained that rootfs= was empty or missing.

If you want to use a prebuilt image, set zbm.hookroot and mask the xhci-teardown.sh script.

Actually, I'd like to use my own images. But since I am no longer able to build working images, using prebuilt images is my last resort. I'll try to understand your instructions and to implement them, and of course I'll report back then.
Best regards,
Binarus

Sorry, I overlooked the fact that you already tried the known-good stub loader.
It's not clear to me whether the dracut emergency shell is appearing before ZFSBootMenu loads or after you've selected an environment from within the menu and attempted to boot.

No problem :-)
ZFSBootMenu does not appear at all with images I've built. It immediately goes into the dracut shell after executing the BIOS. And in that shell, I can't do anything because the keyboard is dead. However, please see my next post :-)
Best regards, and thanks again,
Binarus

If you want to use a prebuilt image, set zbm.hookroot and mask the xhci-teardown.sh script.

This is ingenious!
I have tried it, and finally it works. I can now boot the prebuilt image and have the keyboard available after Debian has booted. Absolutely great - thanks a lot for your help again.
I guess we now can close this issue. I have tested with only one server so far, but I am sure that it will work for the others as well.
I will keep monitoring the other issue. Perhaps somebody explains there why the images I build myself are failing.
Best regards,
Binarus

As a last side note: The servers in question provide a BIOS option where we can enable or disable the XHCI handoff. However, switching that option did not make a difference. Only masking xhci-teardown.sh worked.


Use servo API of bsp package

Use servo API of bsp package
Support the disruptive change brought in the PR below.

https://github.com/mangdangroboticsclub/mini_pupper_bsp/pull/8

previous PR #35


Add ability to import/export type definitions

This was discussed this briefly in IRC (log), and it seems like it'll require some new syntax, likely something to the effect of:
import type { MyType } from "./SomeModule";

This can be easily compiled out of the final output, and use the same path resolution as a regular module import would. The syntax might be a bit confusing when part of a CommonJS module, but on the plus side it has parity with ES6 module syntax.
Rather than add new export syntax, it should be okay to implicitly export all the types in a file (since you have to specify which exactly ones you're importing anyways).

import type has existed for a while now, and has been written about on the blog: http://flowtype.org/blog/2015/02/18/Import-Types.html
Can this be closed, or is there an unresolved issue here?

gonna close, reopen if there's still an issue


Synset pos param

adding pos parameter to .synset(...) method to allow filtering of synsets directly

May need to update lemma methods to use pos param in future


ARROW-13858: [R] Create a field ref to a field in a struct

WIP; outstanding TODOs noted in test-expression.R

Happy to review more as this progresses...just a few observations:

I think ls(x) is not a robust way to check for collisions. You may have to use get() or mget() with inherits = TRUE since the symbol might exist in a parent environment?

For R6 this works because I'm just looking for a method name, and inheritance isn't handled via nested environments it seems (i.e. class_name is defined in the base ArrowObject class but it shows up in ls() for Expression).
We've been using this trick for a while for Tables and RecordBatches: https://github.com/apache/arrow/blob/master/r/R/arrow-tabular.R#L153-L160

Can Exression$field_ref() be updated to to accept a vector? Then you could avoid the C++ compute___expr__nested_field_ref() and do it all in R.?

It could, and that was my first instinct too, but that's actually not helpful for the $ method, where you have a FieldRef and the name of the thing you want to nest further. You'd still have to call out to C++ to get the vector of field names in the path of the existing FieldRef.

There should probably be a way to get all the components of a field ref (maybe compute___expr__get_field_ref_name() should return cpp11::strings()?

Probably. I'm going to focus on getting the essential dplyr stuff working first and we'll see what's required for that.
One further complication that is completely punted here is that FieldRefs can be from strings or integers, so a nested path could contain a mix of integers and strings: https://github.com/apache/arrow/blob/master/cpp/src/arrow/type.h#L1651-L1660 So just switching to std::vector<std::string> everywhere isn't a simple solution.


SAK-32504 - RWiki user with rwiki.create cannot create comments

Looks like I have to fix up a failing test


Ensure that all unmerged files are merged when conflict checking (#20528)

Backport #20528
There is a subtle bug in the code relating to collating the results of
git ls-files -u -z in unmergedFiles(). The code here makes the
mistake of assuming that every unmerged file will always have a stage 1
conflict, and this results in conflicts that occur in stage 3 only being
dropped.
This PR simply adjusts this code to ensure that any empty unmergedFile
will always be passed down the channel.
The PR also adds a lot of Trace commands to attempt to help find future
bugs in this code.
Fix #19527
Signed-off-by: Andrew Thornton<EMAIL_ADDRESS>

:rocket:


quark run should give a helpful message if target is not installed yet

Source ReactiveSocket/reactivesocket#87

Fixed in dbb97f95


The conformance test isn't needed for the unittests.

The conformance.proto is just the communication between the conformance runner and the given language binary, to reuse the tests, the messages are all thats needed, and they aren't in conformance.proto.

Long term we should probably got the tests off the upstream protos files, just so future changes to them don't break us. I did that even upstream for objc (all the test protos are in the objc subdir) and it made this much simpler over the long run.


Compiling vlfeat in android NDK

I know it's not the right place to ask this, though I would really appreciate any help:
I'm trying to compile vlfeat in NDK and I get linking errors:
src/main/jni/vlfeat/imopv.c:137: error: undefined reference to '_vl_imconvcol_vf_sse2'
src/main/jni/vlfeat/mathop.c:485: error: undefined reference to '_vl_distance_l2_sse2_f'
src/main/jni/vlfeat/mathop.c:485: error: undefined reference to '_vl_distance_chi2_sse2_f'
src/main/jni/vlfeat/mathop.c:485: error: undefined reference to '_vl_kernel_l2_sse2_f'
....
this is my make file:
LOCAL_PATH := $(call my-dir)
include $(CLEAR_VARS)
#opencv
OPENCVROOT:= /opt/OpenCV/opencv_2.4.9
OPENCV_CAMERA_MODULES:=on
OPENCV_INSTALL_MODULES:=on
OPENCV_LIB_TYPE:=SHARED
include ${OPENCVROOT}/sdk/native/jni/OpenCV.mk
FILE_LIST := $(wildcard $(LOCAL_PATH)/vlfeat/*.c)
FILE_LIST := $(FILE_LIST:$(LOCAL_PATH)/%=%)
LOCAL_SRC_FILES := com_example_ndk_opencv_androidstudio_NativeClass.cpp
LOCAL_SRC_FILES += $(FILE_LIST)
LOCAL_LDLIBS += -llog -lm
LOCAL_MODULE := MyLib
include $(BUILD_SHARED_LIBRARY)
Any help is really appreciated.
Thanks

Hi, try disabling the SSE instructions (if I remember correctly, you can do this passing ENABLE_SSE=no to make).

On 22 Oct 2015, at 22:16, eicm<EMAIL_ADDRESS>wrote:
I know it's not the right place to ask this, though I would really appreciate any help:
I'm trying to compile vlfeat in NDK and I get linking errors:
src/main/jni/vlfeat/imopv.c:137: error: undefined reference to '_vl_imconvcol_vf_sse2'
src/main/jni/vlfeat/mathop.c:485: error: undefined reference to '_vl_distance_l2_sse2_f'
src/main/jni/vlfeat/mathop.c:485: error: undefined reference to '_vl_distance_chi2_sse2_f'
src/main/jni/vlfeat/mathop.c:485: error: undefined reference to '_vl_kernel_l2_sse2_f'
....
this is my make file:
LOCAL_PATH := $(call my-dir)
include $(CLEAR_VARS)
#opencv
OPENCVROOT:= /opt/OpenCV/opencv_2.4.9
OPENCV_CAMERA_MODULES:=on
OPENCV_INSTALL_MODULES:=on
OPENCV_LIB_TYPE:=SHARED
include ${OPENCVROOT}/sdk/native/jni/OpenCV.mk
FILE_LIST := $(wildcard $(LOCAL_PATH)/vlfeat/*.c)
FILE_LIST := $(FILE_LIST:$(LOCAL_PATH)/%=%)
LOCAL_SRC_FILES := com_example_ndk_opencv_androidstudio_NativeClass.cpp
LOCAL_SRC_FILES += $(FILE_LIST)
LOCAL_LDLIBS += -llog -lm
LOCAL_MODULE := MyLib
include $(BUILD_SHARED_LIBRARY)
Any help is really appreciated.
Thanks
—
Reply to this email directly or view it on GitHub https://github.com/vlfeat/vlfeat/issues/109.

I  tried the following flag to disable the SSE instructions "APP_CPPFLAGS := -U__SSE__" and it didnt help, plus I'm targeting "APP_ABI := armeabi-v7a"

@eicm try to use LOCAL_CFLAGS := -DVL_DISABLE_SSE2 -DVL_DISABLE_AVX for compiling, you can check here and here for some application in android using VLfeat.


Add view ray and labels selection in 3D

Description
In this PR, @alisterburt and I add the ability to select objects in 3D. Following the pattern suggested by @sofroniewn here, we:

modify the layer. _map_canvas2world to correctly map the canvas coordinates to world coordinates for 3D data. Previously, this method used the vispy camera transform. However, I think this may not be the correct transform between canvas and world coordinates for 3D, but based on my experimentation and this comment.
add the view orientation in world coordinates to the mouse event in layer._process_mouse_event()
add a method to transform the orientation vector from world coordinates to layer data coordinates (see here)
add method to base layer to find intersections of the view direction vector with the axis-aligned bounding box of the displayed data (using layer extents)
add a method to the labels base layer to find the first non-background voxel encounted along the layer data view ray ("sample ray").

To show the implementation, we have a example script with a layer that has rotation, shear, and scale applied (see below)

Example code
  """
Display two labels layers in 3D to demonstrate interactive 3D functionality
"""

import numpy as np
import napari

volume = np.zeros(shape=(64, 64, 64), dtype=int)
volume[0:5, 0:5, 0:5] = 1
volume[0:5, 0:5, 59:64] = 2
volume[0:5, 59:64, 0:5] = 3
volume[0:5, 59:64, 59:64] = 4
volume[59:64, 0:5, 0:5] = 5
volume[59:64, 0:5, 59:64] = 6
volume[59:64, 59:64, 0:5] = 7
volume[59:64, 59:64, 59:64] = 8
volume[29:34, 29:34, 29:34] = 9

viewer = napari.Viewer()
volume_1 = viewer.add_labels(volume, name='volume 1')
volume_2 = viewer.add_labels(
  volume,
  name='volume 2',
  scale=(3, 2, 1),
  shear=(0, 0.1, 0),
  translate=(64, 0, 0),
  rotate=(90, 0, 0)
)

def click_drag(layer, event):
  near_point, far_point = layer._cursor_ray(event)

  if (len(near_point) > 0) and (len(far_point) > 0):
      print(near_point, far_point)
      value = layer._get_value_ray(near_point, far_point)

      if value is not None:
          print(f'selected label: {value}')

volume_1.mouse_drag_callbacks.append(click_drag)
volume_2.mouse_drag_callbacks.append(click_drag)

napari.run()

https://user-images.githubusercontent.com/1120672/125492895-e9d36534-1c30-4f4b-a4f8-f96def7493af.mov
Some things to improve:

currently, the algorithm we use to construct the ray passing through the data cube requires the transform mapping canvase coordinates to world coordinates. Currently, this is not available in the layer and it would be nice if it was because it makes it difficult to do interaction in 3D. Fixed,  see here
we aren't totally sure about where to distribute the various functions (e.g., on the viewer vs. on the layer vs. utility functions).

Type of change

[ ] Bug-fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

References
Closes #2653
How has this been tested?

[ ] example: the test suite for my feature covers cases x, y, and z
[ ] example: all tests pass with my change

Final checklist:

[ ] My PR is the minimum possible work for the desired functionality
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] If I included new strings, I have used trans. to make them localizable.
For more information see our translations guide.

❤ ❤ ❤
https://user-images.githubusercontent.com/7307488/125629307-fc0958ee-3bc1-45d5-a278-11486673c566.mp4

Super cool, @alisterburt . If others want to try it, see the cursor_ray.py example in this PR. I think it's worth noting that the mouse callback is just:
@volume_layer.mouse_drag_callbacks.append
def on_click(layer, event):
    near_point, far_point = layer._cursor_ray(event)
    ray_points = np.linspace(near_point, far_point, n_points, endpoint=True)
    if ray_points.shape[1] != 0:
        ray_layer.data = ray_points

Thank you for the feedback , @jni and @sofroniewn ! I think we are pretty much there in terms of the core code and we now just need to decide how to hook it up.
Just to make sure we're all on the same page, a quick summary:

The mouse event how has the following properties:

pos: the position of the click in canvas coordinates
position: the position of the click in nD world coordinates
view_direction: the vector point in the direction of the camera into the scene in nD world coordinates
dims_point: the indices of the current slice (from viewer.dims.point)
dims_displayed: the ordered dims currently displayed in the (from viewer.dims.displayed)

We added a vector_world_to_data() which transforms a vector from world to data coordinates, preserving the orientation.
We added cursor_ray() to the base layer, which takes a MouseEvent and returns the start and end point for the ray that intersects the  in nD layer data coordinates.
On the Labels layer, we've added a _get_value_ray() method, which takes the start and end point of a ray in nD layer data coordinates and returns the first non-zero (i.e., non background value) encountered.

In terms of what I think users/developers will want to interact with:

I think developers will want to use layer.cursor_ray() to get the click ray that goes through their data so that they can add custom 3D interactivity
I think users will want to be able to select and move items in 3D.
I think users will want to be able to inspect their data in 3D (i.e., status bar should update with appropriate value for what is under the cursor).

To achieve this, I propose the following:

We modify the cursor_ray() signature to instead accept the values it requires as individual arguments (i.e., click_position_world, view_direction_world, and dims_displayed) instead of MouseEvent. This allows people to use it outside of mouse event callbacks (e.g., interrogating their data for some analysis). Also, potentially rename?
We modify layer.get_value() to accept click_position_world, view_direction_world, and dims_displayed. Based on these values, get_value() can correctly call the 2D or 3D get_value() function. We can make this a non-breaking change by makingview_direction_world and dims_displayed optional and if they aren't provided, it falls back on the current get_value implementation. We can implement the 3D get_value() function for labels in this PR and add it for the other layers in future PRs.
To correctly update the status bar, we modify get_status() to pass click_position_world, view_direction_world, and dims_displayed to layer.get_value() so the 3D get_value can be called (when appropriate). This means that we also have to update wherever get_status() is called so that it can pass click_position_world, view_direction_world, and dims_displayed.
We rename layer.vector_world_to_data to layer.ray_world_to_data or layer.direction_world_to_data to avoid confusion with the Vectors layer (it's probably a misnomer anyway - see next point).
I would prefer to keep layer.world_to_data and layer.vector_world_to_data (whatever we rename it to) separate. They are fundamentally different transformations. layer.world_to_data maps positions and layer.vector_world_to_data maps the orientation of the vector (e.g., layer.vector_world_to_data doesn't apply translations).

I think this is pretty in line with what @sofroniewn and @jni  suggested, but I wanted to make sure we are all on the same page because I think we are going to want to build a lot on top of the functionality. What do you all think?

I think this is pretty in line with what @sofroniewn and @jni suggested, but I wanted to make sure we are all on the same page because I think we are going to want to build a lot on top of the functionality. What do you all think?

All this sounds pretty good

The mouse event how has the following properties ....

I'm a tad nervous about this as it's somewhat funny to me that the mouse now knows about some of this dims stuff, but on the other hand I can see why it's needed and I don't really see another way right now.

We modify layer.get_value() to accept click_position_world, view_direction_world, and dims_displayed ...
This means that we also have to update wherever get_status() is called so that it can pass click_position_world, view_direction_world, and dims_displayed

Do you want to make a small dedicated PR that makes just these changes (without view_direction_world). Might be easier to review and keep this diff a little smaller.
I saw @kevinyamauchi you've got some good tests inside the labels layer where you mock the mouse event. Can you make a copy of those tests that test the same stuff but put them at the level of the whole viewer and trigger the mouse event manually - I want to make sure that if we changed something like the addition of the mouse_event.dims_displayed we'd get a fail, and i'm not sure if that's already the case.

Thanks for the feedback, @sofroniewn !

I'm a tad nervous about this as it's somewhat funny to me that the mouse now knows about some of this dims stuff, but on the other hand I can see why it's needed and I don't really see another way right now.

I agree it's a little funny. I also don't see another way at the moment and I don't think bad, as it's not part of the Cursor model, it's just added to the event by the Viewer to aid in the interpretation of the position and view_direction (I think correctly interpreting even just the cursor position is difficult without the dims_displayed and dims_point).

I saw @kevinyamauchi you've got some good tests inside the labels layer where you mock the mouse event. Can you make a copy of those tests that test the same stuff but put them at the level of the whole viewer and trigger the mouse event manually - I want to make sure that if we changed something like the addition of the mouse_event.dims_displayed we'd get a fail, and i'm not sure if that's already the case.

Good call. Will do!

Do you want to make a small dedicated PR that makes just these changes (without view_direction_world). Might be easier to review and keep this diff a little smaller.

I think this is a great idea! This PR is definitely getting a little bulky. In that case, the last thing is to decide on

I think this is a great idea! This PR is definitely getting a little bulky. In that case, the last things are:

I guess that PR could also come after this one too. Happy to do it either way

to decide on the names for what are now cursor_ray() and layer.vector_world_to_data

How about _cursor_ray -> get_ray_endpoints, and vector_world_to_data -> world_to_data_ray

Some minor comments on the discussion above:

How about _cursor_ray -> get_ray_endpoints, and vector_world_to_data -> world_to_data_ray

_world_to_data_ray? But also, how about _world_to_data(point=..., direction=None)? If I follow the discussion above (perhaps I don't!), the issue is that transforming a point and a vector are two different things. But both versions of the function require transforming a point. By giving direction=None, we remove that part of the code in 2D, but use it in 3D.
But, whatever you decide @kevinyamauchi, let's keep it private still, so we can refactor to our heart's content after merge. 😊

I think developers will want to use layer.cursor_ray() to get the click ray that goes through their data so that they can add custom 3D interactivity

Why not update the single Cursor model with position to be equal to the front-face position in 3D and position_end equal to the back face in 3D, or None in 2D? (Or it could be identical to position in 2D.)
Other than that, in general terms, I absolutely love the proposal to make methods less dependent on global state.

btw @kevinyamauchi feel free to ping me on Zulip this morning if you work on this and want to bounce ideas off someone. I don't have a pair session scheduled tonight. 😊

Why not update the single Cursor model with position to be equal to the front-face position in 3D and position_end equal to the back face in 3D, or None in 2D? (Or it could be identical to position in 2D.)

Yeah that could be nice. Might want to keep the additional cursor properties private until we figure this out too, but we can always deprecate if we change

Why not update the single Cursor model with position to be equal to the front-face position in 3D and position_end equal to the back face in 3D, or None in 2D? (Or it could be identical to position in 2D.)

@jni and I discussed this on zulip, but I'll just summarize here in case anybody is curious. It is challenging to put the front/back intersections on the Cursor model because the Cursor model lives on the Viewer. Since the Cursor model is on the Viewer, the front/back face should probably be with respect to the bounding box of the full scene. Currently , we do not have a concept of the scene extents on the Viewer (as far as I know). While not so difficult to implement the scene extents, I propose that add/discuss this in a future PR.
After @tlambert03's  and @sofroniewn 's feedback and some conversations with @jni and @alisterburt , I have made the following changes:

added a test to the qt_viewer to test the properties that are added to the MouseEvent
renamed layer.cursor_ray() -> get_ray_intersection()
renamed vector_world_to_data() -> _world_to_data_ray()
revised the _process_mouse_event() docstring

With all of that, this PR gives the ability to:

in a mouse click callback, get the intersections of the mouse click ray with the data bounding box for all layers
adds a function to get the first non-background label along a ray to the Labels layer

In the follow up PRs, we will:

hook up the 3D interaction to layer.get_value and layer.get_status
add 3D selection in the GUI to points, labels, and shapes

I think this is pretty much ready to go! Looking forward to any final feedback you all might have!

Nice work here @kevinyamauchi! Let's get this in!! 🚀
I'm approving now, but will probably let sit one more night in case anyone else wants to review/ has additional feedback - otherwise will plan on merging tomorrow morning. How does that sound?

Sounds good to me! Thanks everyone!

Currently , we do not have a concept of the scene extents on the Viewer (as far as I know). While not so difficult to implement the scene extents, I propose that add/discuss this in a future PR.

Do you want to use viewer.layers.extent? Would that help? (sorry not sure on details here)
https://github.com/napari/napari/blob/529181d20efdcf300233dd89722c99f37dbe5ffd/napari/components/layerlist.py#L236-L244
No need to use that now, but if that's useful and you want to change in this PR let me know - especially if it would change any public API that we're about to create

@sofroniewn , cool! I didn’t know about that. It doesn’t affect anything in this PR, but it may inform how we hook the cursor model in the follow up PR. Thanks!

@kevinyamauchi @sofroniewn note that we've had lots of performance issues with viewer.layers.extent, so I would stay away from it for these purposes until we've started caching layer extents.

@kevinyamauchi @sofroniewn note that we've had lots of performance issues with viewer.layers.extent, so I would stay away from it for these purposes until we've started caching layer extents.

Ah yes - good point!

Would be worth knowing though it if would be highly useful here though, and the we should cache layer extents - rather than just reimplementing the same thing in parallel

Indeed, we should not reimplement. But I think what would be useful here is viewer.layers.selection.active (from memory, might be different).


fix: Avro LogicalType time-millis is in milliseconds.

What is the current behavior? (You can also link to an open issue here)
When converting time values, it is currently saved as nanoseconds since midnight.
What is the new behavior?
According to the Avro spec, for this logical type it should be saved in milliseconds since midnight.
Please check if the PR fulfills these requirements

[x] The commit message follows Talend standard
[ ] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been added / updated (for bug fixes / features) ?
[ ] The code coverage on new code >75%
[ ] The new code does not introduce new technical issues (sonar / eslint)

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build / CI related changes
[ ] Other... Please describe:

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information:

Pull request test result: success :star:
Sonar results


iso-2022-jp should be handled as ISO-2022-JP in unstructured fields.

Otherwise, Encoding::CompatibilityError is raised in the following program:
require "mail-iso-2022-jp"

mail = Mail.new(charset: "iso-2022-jp")
mail["User-Agent"] = "test mailer"
p mail["User-Agent"].encoded #=> Encoding::CompatibilityError

:green_heart:

Thank you!


chore: GCForms release v3.1.1

:robot: I have created a release beep boop
3.1.1 (2023-06-19)
Miscellaneous Chores

deps: lock file maintenance (#2298) (78b36c6)

This PR was generated with Release Please. See documentation.

:robot: Release is at https://github.com/cds-snc/platform-forms-client/releases/tag/v3.1.1 :sunflower: