Make adding Collaborators requiring confirmation on the invited collaborator

Feature Description
As of right now, adding a Collaborator to a repository is done so without any kind of acceptance on the invited user's end.
I see this as a risky thing to have. While it may make things easier in terms of adding new collaborators can it also increase the risk with users being added to repository they don't want to be. Especially when the user in question is automated for certain features.
To give a personal example: I use Renovate with a dedicated user account to keep my dependencies updated.
One feature it has is "autodiscover" meaning it automatically detects and manages dependencies of repositories the Account has write access on.
I was interested in that feature as it would allow me to not having to constantly add new repositories to keep track of, but with how the collaborator adding works in Gitea is this a too high risk as users could simply add the account to their repository, causing higher usage of the backend-software and perhaps even API rate limits or similar.
Having the invitation of collaborators require a confirmation by the invited user would reduce this risk as users couldn't just add the account to whatever.
The invite should probably expire after 24 hours or a similar timeframe to not waste storage space on the database, as low as it may be.
I also tested this on try.gitea.io and the problem exists on it.
Screenshots
No response

GitHub initially also had this without confirmation, but now they do. I think it's definitely needed.
IIRC, what GitHub does is create a notification for the invited user, which presents them a simple fullscreen page:
               Username has invited you to org/repo

                [Accept (green)] [Decline (gray)]

One issue I forsee is bot accounts. When inviting a bot account, they must automatically accept the invitation because a bot account should be assumed to not be logged in by a user who could confirm this dialog.
While a invitation is pending, the collaboration status should show "Pending Invitation".

One issue I forsee is bot accounts. When inviting a bot account, they must automatically accept the invitation because a bot account should be assumed to not be logged in by a user who could confirm this dialog.
While a invitation is pending, the collaboration status should show "Pending Invitation", ideally with a tooltip indicating time until expiry.
24h expiration is too short and would be annoying. I would at least make it 10 days, for example when a user is on short absence. Find out what the expiry time on GitHub. Likely make it configurable.

Maybe the invitation should be sent via both email and UI notification.

Maybe the invitation should be sent via both email and UI notification.

That would be the best solution here. Maybe also allow a user to define in the settings whether they only want notifications or also e-mails for invites.
Also, to clarify, I assume you mean automated user accounts with "Bots"? Because for me Bots are actual, dedicated bot accounts and not user accounts with automation.

Yes, I mean automation accounts that I create for deployments. They have an app token to clone repos, but otherwise they never log in. I guess we would need a flag on these accounts in the admin panel to mark them as such, maybe "Non-user account"?


Synchronize module partially broken

Sadly the tasks that include copying data back from a host to the Ansible Controller via synchronize module can be broken. The error message says something like:
 no tty present and no askpass program specified

Especially that behavior seems to occur, if remote servers do NOT have passwd-less entries for rsync.
A fix that I found to work (so far) is simply to amend the module call with the rsync_path flag like
- name: Copy Generated SSL Files Back to Ansible Host
  synchronize:
    src: /var/ssl/private/generation/out/
    dest: "generated_ssl_files"
    mode: pull
    rsync_path: /bin/rsync

The source of the workaround is to be found here:
https://github.com/ansible/ansible/issues/20769#issuecomment-546438205
I am aware that confluent.common already contains a task to anticipate problems with synchronize
- name: Disable Require TTY in Sudoers (required for ansible synchronize)
  lineinfile:
    path: /etc/sudoers
    state: absent
    regexp: '^Defaults\s+requiretty.*$'

However I do have customers that do central Server-Basis Management with Puppet including their sudoers file. Meaning after 30 min the task is sadly obsolete :P
Did anybody every experience similar issues?

I have not. The only thing that seems concerning is the assumption that the rsync binary lives at /bin/rsync on all linux hosts. Is there anyway to confirm this? We will be doing testing across ubuntu, debian, and centos/rhel before the 600 release. Maybe that would be enough.
Another option is replacing synchronize with the fetch module, which is pretty much the inverse of copy.

One one to definitely get the correct path would be to look it up beforehand
- name: Lookup rsync path
  command: which rsncy
  register: rsync_path
[...]
  rsyncpath: "{{ rsync_path.stdout }}"

I am not sure however whether that is an overkill :smile:
Fetch does work, however only for single files (not sure how much fun it is to loop the content of a dir and fetch it file by file) - I did not check now if there is the need to sync a whole dir somewhere in cp-ansible, but that would probably be the easiest and most stable solution :+1:

I mean regarding using fetch and a loop, we have the ca cert generation code, we should know exactly which files to pull back.
The one thing I like about synchronize is that it creates the generated_ssl_files directory for you. Idk if fetch will do that, but maybe we can include that dir in git to solve that problem.

@Fobhep Can you please review https://github.com/confluentinc/cp-ansible/pull/392
I think that is the only synchronize task in the entire install. There are some in our test roles, but those do not need to be changed because customers do not run them

@Fobhep I am going to go ahead and close this out, as the change was merged.  Please feel free to re-open if you feel this is incorrect.


STREAMS-646 support new Multi-Field requests in streams-processor-fullcontact

I'm satisfied with this from local testing
There's another PR coming for STREAMS-645 integration testing for streams-processor-fullcontact
Containing integration tests that will demonstrate it works, BYO API token

i will lazy consensus merge this in 24 hours


add code owner

Description
add myself as codeowner on edge-frontend repo
Fixes # (issue)
Type of change
What is it?

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Documentation update
[ ] Tests update

Checklist:

[ ] 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
[ ] New and existing unit tests pass locally with my changes
[ ] I run npm run lint:js:fix to check that my code is properly formatted

Codecov Report
Patch and project coverage have no change.

Comparison is base (5f2f200) 27.83% compared to head (e471f16) 27.83%.

:exclamation: Current head e471f16 differs from pull request most recent head 0496d30. Consider uploading reports for the commit 0496d30 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #695   +/-   ##
=======================================
  Coverage   27.83%   27.83%
=======================================
  Files         130      130
  Lines        3291     3291
  Branches     1138     1138
=======================================
  Hits          916      916
  Misses       2144     2144
  Partials      231      231

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


Inactivity changes to hide recently-approved users

Also bump time to 90d.

The way I'd like to do this would be to do all the checking in the database, and pull out approval date, last active, username, etc in one query that did everything necessary. However, the code at present relies on PHP logic to get the current on-wiki username, which can't be done in the database, and requires user objects be pulled back from the database, thus we can't add extra fields to it. As such, we're stuck with either rewriting the oauth code (which is what I did in newinternal), or doing horrible hacks like pulling the approval date for every returned user separately, in individual queries. This is how the existing code works.
My change admits that this is bad, and does nothing to fix it.
Instead, I add to the logic in the display template to simply hide the suspend button from recently approved users, in the same way that it's currently hidden for inactive checkusers.
Using my data-sanitised production clone, and using James as a test user:

Codecov Report

Merging #461 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #461   +/-   ##
=======================================
  Coverage   20.54%   20.54%
=======================================
  Files           9        9
  Lines         584      584
=======================================
  Hits          120      120
  Misses        464      464

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b936064...2cdb48f. Read the comment docs.


Unable to select first recipe in the list in cooking menu

First of all - thanks for the amazing mod.  I'm running client version <IP_ADDRESS>.  I have noticed that I am unable to select the first recipe on the cooking menu.  It just returns me to the campfire menu, but doesn't cook the meal.  If I add another recipe which pushes the previous one down the list, then it works fine from position two in the list.
I'm running the following mods...
QudUX 2.0
You see a cave spider and that's ok 1.0.1
Colors of Qud 1.3
Let me know if you need any more information or need be to try anything

Fixed in f0dbc55c79e9010df29bb11cc5bd4a757b00c2f6. Thank you for the report!


Trident throw sound ("whoosh") missing

Describe the bug
The trident "throwing" sound is not playing. The sound after hitting plays fine, when coming back (with Loyalty) plays fine, just the sound that should play immediately after the trident leaves the players' hands isn't playing.
To Reproduce
Steps to reproduce the behavior:

Install VMP (I did it through a Prism Launcher instance, brand new, created just for this test, on 1.20.1 with Fabric Loader 0.16.3)
Grab a trident from creative inventory (on a new world)
Throw trident
Notice how "empty" it sounds without the throwing sound
If wanted, go back and disable VMP, open again and throw the trident. It's clear the difference.

Runtime info (please complete the following information):

OS: Windows 10
Minecraft version: Java 1.20.1
Mod version: 0.2.0+beta.7.102

Other mods
First diagnosed with a personal performance modpack, but reproduceable with only VMP installed.
Checklist

[X] I am using the official version of the mod.
[ ] I tried the latest development version but the issue persists.    -- Happens on the latest available for 1.20.1
[X] I searched for similar open issues and could not find an existing bug report on this.

Additional context
Guess that's it.

Is it reproducible on 1.21.1?

It is not. Hadn't tested before as I have no interest in 1.21.1 (yet), as the server with my friends isn't ready to move until we have all mods ready.


CanCOGeN template: enable null values for Sequencing Instrument

In the DH CanCOGeN template, there is a pick list for Sequencing Instrument. It is a multi-select.
The DH template shows that null values should be available (in the data status field), but they don't show up in the pick list.
Can we make sure these are an option?
We just had an instance where some datasets had to supply a null value for now but will be updated later, and we had to leave to blank which will cause an error during validation.

Looks like null value selections aren't being added to multi-select fields.

Is there a way to fix that? (asks she hopefully)

yes, its a bug!  Will resolve shortly.

Champion! (as my dad would say)

Fixed in latest draft.


Missing responsive navbar?

I'm unable to find any particular way through which I can make a responsive navbar. Is that component available here?

Menus can collapse to take up only as much space as needed, stack on mobile devices, stretch to take up the full width of their container, and evenly divide their items.
http://react.semantic-ui.com/collections/menu#compact
http://react.semantic-ui.com/collections/menu#stackable
http://react.semantic-ui.com/collections/menu#fluid
http://react.semantic-ui.com/collections/menu#evenly-divided
Additionally, you can put a Menu in a Grid for even more responsive control:
http://react.semantic-ui.com/collections/grid

Links provided are broken
https://react.semantic-ui.com/collections/menu#menu-example-compact
https://react.semantic-ui.com/collections/menu#menu-example-stackable
https://react.semantic-ui.com/collections/menu#menu-example-fluid
https://react.semantic-ui.com/collections/menu#menu-example-evenly-divided

Little outdated, but I've made a codepen that shows how to make it.

Haha, boy, it seems my response got mix feelings.  I have no idea why, but it sure does make me curious.

Somewhat unrelated, but, why is the semantic ui react documentation not responsive at all? When I visit on my phone, i have to pinch and zoom everywhere, unlike the CSS doc site.
I'm a long time fan of semantic ui, wrote a yeoman extension for it even for .NET Core project generation (the CSS version). But I've had problems getting the React version to be responsive, or maybe it's because of my lack of understanding of the proper pieces that must be used to make it responsive.

@jackjwilliams All components that are responsive in SUI are responsive in SUIR. Item, Form, and Grid can all be defined with responsive props which render them with the proper SUI markup for collapsing, doubling, etc.
Regarding the docs... no. They are not responsive. That would probably be a good addition to the project which we could make a separate issue for if one does not already exist.

@jackjwilliams and @brianespinosa - I noticed that the docs were not responsive a while ago and created an issue for this #2230. Still waiting for the fix to push.
Thanks for the example @layershifter, any chance that could be added as an example to the docs, maybe in the layouts area?

@veswill3 @jackjwilliams looks like #2331 was merged 3 days ago so I assume that means when the next release of SUIR goes out we will get the necessary viewport tags added to the docs.

This sounds like a feature request, please open a new issue.


Convert "SIG Scalability - Kubernetes The Hard Way" blog to Markdown

This is a post contributed by Alex Handy (@VonGuard) that I'd like help reviewing and making sure it looks nice before it gets posted!
Thanks for your contribution, Alex!
See the previous PR for reference.

/hold

/sig contributor-experience

@mbbroberg, you rock! Thank you for the feedback! I'll get those items addressed later today for another review!

@mbbroberg and @mm4tt - let me know if that looks good to you, and if so, I'll round up some final approvals!

@sftim - can I get your look on this, too?

@parispittman - updated! Thank you for that catch.

/lgtm
Remove the hold at your convenience, thanks!

/approve

Thank you for all of your help and reviews on this!
/hold cancel


NPE when calling zone.getLongStackTrace();

When calling zone.getLongStackTrace() from console (during debug). The method throws an NPE because exception is not specified. It would be useful to make the exception optional so that it would be easy to query the current location.
The issue is in long-stack-trace-zone.js#L50
A simple check to ignore null exception should suffice.

:+1:


Performance Goals

I'd really like to see some performance improvements. Let's set some goals for performance.
What do you think would be things to measure performance.
I suggest using some of these metrics

Frontend Page Load (Time)
Student Assignment Dashboard (Time)
Client -> Server Submission (Time)
Client -> Server Backup (Time) - Ideally have the server respond before the timeout. We can't solve all cases of this - but we should try to.
Staff -> Dashboard Load Times. (Time) - Takes over 15 seconds now
Staff -> Check for individual Queue (Time)  - Not even on front-end currently
Staff -> Load All Queues (Time) - Takes over 15 seconds
Staff -> Load A Queue (Time) - this is pretty fast already

Things currently being done to improve performance

Upgraded instance size.
More caching on the student frontend, so loading indicators show up less frequently.
Caching on the staff side
Utilizing memcache more for queries.
Aggressive time out on the client (though this results in more user facing errors)
Memcache was sitting at its 1GB limit - I've bumped the size - so we'll also see if that helps with performance.

Can we combine this into "Average page load time"? Eventually, we want a weighted average so that pages that are accessed more frequently get more weight in the average, but... I think the former is better.

Totally agree. This would be great to keep track of.
I think there's a new feature called "Traces" in app engine that helps you
out with this. @denero can you enable that somehow?
also, we should look at percentile load times. I remember that was standard
practice for my internships when they looked at performance. See what the
90th percentile load time is rather than just the average.
On Thu, Feb 26, 2015 at 1:27 AM Some3141<EMAIL_ADDRESS>wrote:

Can we combine this into "Average page load time"? Eventually, we want a
weighted average so that pages that are accessed more frequently get more
weight in the average, but... I think the former is better.
—
Reply to this email directly or view it on GitHub
https://github.com/Cal-CS-61A-Staff/ok/issues/401#issuecomment-76146727.

I did not find a way to enable Traces in AppEngine. Do you have instructions somewhere?

Traces is already enabled - it's under Monitoring on the developer console.

v2 is no longer being maintained.


Can't create a new migration with fresh clone of headless-cms

There is some kind of eventLog problem.

Updating dependencies did the trick for me.

@FilipChalupa We've published new versions of Starter kits with access control decorators and more and fixed this issue.


version CURL_OPENSSL_3 not found - on Raspberry PI 4 - Ubuntu 20.04

Agent Version and Platform
Version of your agent? 2.194.0-prerelease & 2.193.1
Raspberry PI 4B 8GB - Ubuntu 20.04.3
Linux ubuntu 5.4.0-1042-raspi #46-Ubuntu SMP PREEMPT Fri Jul 30 00:35:40 UTC 2021 aarch64 aarch64 aarch64 GNU/Linux
Azure DevOps Type and Version
on-premises DevOps Server?
Version Azure DevOps Server 2020 Update 1
What's not working?
I have a fresh installed Raspberry PI 4B 8GB (with a PoE Shield) and runs Ubuntu 20.04. I downloaded the Linux ARM64 2.193.1 and 2.194.0 agent from GitHub and folled the steps below the release post.
Also i run sudo apt update & sudo apt upgrade and then i used ./bin/installdependencies.sh with root privilegs to install all dependencies.

Are you having any issue related to "WARN VisualStudioServices] Authentication failed with status code 401.
Cache-Control: no-cache"?
I am sorry, I know it is another issue, but I could not find another person who does user rpi 4.
Thank you!

Hi @WanielDeiss could it be related to this one - https://askubuntu.com/questions/1087576/version-curl-openssl-3-not-found?
Do you have any other libssl versions installed on your environment?
@rockrilvas this seems to be some other not related to this one.. Could you please open a separate ticket to take a look?

I'm closing this due to inactivity, feel free to request a reopening if you have further questions.


Enable new listkeys backpressure for new clusters, by default

If https://github.com/basho/riak_kv/pull/263 is accepted, this patch will enable the new behavior by default.

+1 to merge


exposition.generate_latest() is slow

With just the default metrics of the Python client, this function takes around 2 ms on my old-ish laptop.  I'm sure it would rise to 5-10 ms once the app starts adding metrics.  Given limited options due to the GIL, it's really not acceptable to halt soft real-time applications for that amount of time on every metrics pull.
Upon profiling, I noticed some low hanging fruit:

floatToGoString() is 18% of total time.  I think it could be replaced with Python's "g" format.
sample_line() local function is just slow - 13% of total time in its own code.  Worth trying to optimize.
str.format() is called ~500 times for each call to generate_latest(), representing 12% of total time.  f-strings would probably do wonders.

1: Not quite, we need a slightly different format
3: f-strings are not available in Python 2
I'm happy to look at optimisation PRs.

'g' format has precision control, so the 'f' vs. 'e' switching could be made to match Go's 'g'.  From there I think it's just transform of "inf" -> "Inf", etc.?

my plan for throttling CPU use of generate_latest() without modification to client_python:

given that generate_latest() only uses the collect() method of input registry
wrap the collect method
after every ~500 usec wall time, collect iteration will sleep for ~50 ms  (so 1% CPU cap)
assuming that generate_latest() can be called from a background thread

That won't work if there's an exporter written in Python, and would slow some things down quite a bit.

Would you explain how it would break things?
The sleeping wrapper would only be on the registry object passed to generate_latest().  Anything else using the registry would get the normal collect() behavior.

generate_latest is how it's usually used, including by exporters.

I guess in my case I have a custom exporter, calling generate_latest() directly to serve the text on an asyncio web server.  I'm not aware of anything else which would use generate_latest().

Most users wouldn't even be aware that generate_latest is being called, as it's usually done under the covers.

Sorry for the confusion.  To be clear, the scheme of adding sleep during collect iteration is just something I would do locally for my needs to work around the issue.  I landed on that because it doesn't require maintaining any patches to client_python (it's merely fiddling with the object passed to generate_latest).
As far as a more general improvement that could be submitted, I'd suggest revisiting whether clients must match Go float formatting in such exacting detail.  I'm sure Go's float parser can handle both 1234567.8 and 1.2345678+e06 equally well.    If floatToGoString() were simplified to just inf and nan case changes (if that's even needed), it would be a big win.

Demonstrating that golang float parsing is not particular about fixed vs. exponent notation, nor case, nor exponent leading 0.  The floatToGoString() translation doesn't appear to be needed-- am I misunderstanding?
inputs := []string{"1234567.9", "1.2345679e+06", "1.2345679e+6", "inf", "-inf", "nan"}

1.2345679e+06
1.2345679e+06
1.2345679e+06
+Inf
-Inf
NaN

https://play.golang.org/p/MvmKCnPsyA_8

It's required for some openmetrics stuff, and this code is the reference implementation. Thus it uses it everywhere it avoid confusion for other implementors.

openmetrics explicitly requires 1.2345679e+06 rather than 1234567.9?  Has anyone questioned that?  It's inefficient for scripting language clients to implement custom floating point to string conversion.

It's only required in certain places, and it's more a question of if it's possible than efficiency.

It's only required in certain places

Why would openmetrics specify the exact point of switching between fixed and exponent notation, rather than parse either notation?
I don't see any detail about this in the openmetrics draft (https://github.com/OpenObservability/OpenMetrics/blob/master/markdown/metric_exposition_format.md)
The Prometheus docs say only that the number is able to parse by Go's ParseFloat() (https://prometheus.io/docs/instrumenting/exposition_formats/)

it's more a question of if it's possible than efficiency

It's heavy for clients like Python or JS to do such custom number formatting, and specifically for Python represents about 20% of text generation time.

I don't think there's anything we can really do here, so closing off.


Update camranger to 1.8.7

Thank you for the contribution. It was merged directly as af53bbc9309fc2edc6f7d46eafceb8528fd11a4a to keep commit history cleaner. Your contribution is still credited to you.


Fix startIndex Calculation

Using startIndex of the outerDict is wrong. We go to the trouble of
searching for the first non-empty inner dictionary for a reason. We need
to its that index as the start instead.

@swift-ci test


/v17.06/ thinks it's current and has no archive butterbar

https://docs.docker.com/v17.06/
The drop down shows "current" on 17.06 (doesn't have a concept of 17.09) and does not have the archive butterbar.
We must have missed a step for ensuring that the json got updated?

I'm investigating.

Fixed via #4787


Add Environment.java to AIMA4e

This PR consists :

Add Environment in README.md.
Add Environment.java.

solves issue #371

The JavaDoc comments in 0229025 are incorrectly placed.

Sorry, @okulev  I am not able to understand. Can you please point out the improvement?

The JavaDoc comment of addAgent method should be JavaDoc for getAgents. JavaDoc of removeAgent should be moved to addAgent, and so on.

Thanks, @okulev  for the correction. I corrected it


Mark an item as in progress on roadmap which was accidentally marked as completed

per @nikhiljindal 's comment on https://github.com/GoogleCloudPlatform/kubernetes/pull/7041

Thanks, LGTM


Add TheMartianRockets

Closes https://github.com/KSP-CKAN/NetKAN/pull/2379

LGTM! Merging with Thanks! :D


Refactor gnarly Ecto query

I opened this discussion on Elixir forum months ago and nothing there competent sorted it out for me it has a blob of raw SQL that would vastly improve efficiency of a big query in the app, but never figured out a satisfactory solution other than maybe one or more SQL fragments.  Maybe there's an option other than group by, like a sub query or something?  Here's discussion and context: https://elixirforum.com/t/sql-vs-ecto-optimization-translation-help/5559
There's an accidentally checked in second version of the query called "play" that helps a little by executing just the query and not some subsequent Elixir data manipulation, so that may be useful for improving the performance.

I gave this one a try because it seemed like an interesting challenge. :)
I used explicit joins in order to be able to select specific columns to reduce the amount of fetched data.
My dev environment doesn't have a lot of data right now, so I could not test the query in more complex situations, but it seemed to work and was 3 times faster than the previous query.

Just responded over in the PR, thanks man!


NEW: adds lineplot

okay some philosophical thoughts on how functionality should work here - re-thinking this as being similar to scatterplot (because it's not as similar as i originally thought).

user will input x and y measures (both must be numeric MD cols), and the chosen x measure will be ordered.
there will be a 'grouped' param that will default to False, and will take a single categorical MD col as input.

if the 'grouped' param is False, all values in x measure will be ordered (and maybe error if any identical values are found?) and then plotted against the y measure.
if the 'grouped' param is utilized with the particular categorical column, x and y data will be grouped accordingly and then each subgroup of x will be ordered (to create the separate line faceting).

my instinct here is to make this less of an exploratory plot than w/scatter (where you can utilize drop downs to view all different groupings of x/y data) since this feels like there's more intention required to view something meaningful. also open to ideas about why the drop downs should be retained (i could totally be missing why that would make sense).

Hi @lizgehret I see what you mean that this is not as flexible as a scatterplot. Similar to the scatterplot:

x and y must both be numeric
a categorical variable (or numeric variable) can be used for grouping (coloring and curves here; coloring only, but possibly also eventually other options in scatterplot)

It seems like the main constraint is that the grouping variable should have multiple values of x to create a line/curve. So what if the following is done:

all numeric metadata columns can be used for y and are added to the drop-down menu for y.
all numeric metadata columns passed as input are screened to see which columns have at least 2 unique values. This forms the drop-down options for x. (but I have an edge case to think about*)
all categorical and numeric columns that have at least 2 unique values form the options for "grouping"

if the 'grouped' param is False, all values in x measure will be ordered (and maybe error if any identical values are found?) and then plotted against the y measure.

There should not be an error if there are identical values of x — whether grouping is True/False we would expect that there could be replicate values (e.g., biological or technical replicates), this would be typical in many experiments. In fact, these could/should be used (when available) to calculate a confidence interval that could be toggled on/off.

my instinct here is to make this less of an exploratory plot than w/scatter

I agree. Scatter is very exploratory. Line/curveplots less so, but still to some degree. Here are some concrete cases where we would want to use this in an exploratory fashion and give flexibility with drop-downs:

passing multiple alpha vectors alongside numeric metadata; users may want to click through the different metrics to display on the x-axis for visualizing correlations with some numeric variable x. (basically, a replacement for the basic alpha correlation plots we have now). Other variables (e.g., PC coordinates) could also be passed for y, so this could enhance this to allow alpha or beta correlation.
same for alpha rarefaction (though currently this is a box + line plot, so maybe needs a different visualization, but I think that a curveplot + confidence interval would be a fine representation)
In other plugins like q2-quality-control, RESCRIPt, etc there are some actions that create multiple different scores that could be displayed on a curveplot. Right now q2-qc has static line plots showing accuracy at multiple depths; RESCRIPt uses q2-longitudinal's volatility plots to do also display accuracy at different taxonomic ranks.

To contrast: we have q2-longitudinal's volatility plots for less exploratory more structured cases where a user wants to display different y variables against a fixed numeric x variable; categorical grouping variables can be selected from a drop-down, and optionally a fourth (fixed) variable can be used to define individuals within groups that had repeated measures taken. q2-vizard's curveplot should be more flexible and exploratory than this. Users who have, e.g., temporal data should already be using q2-longitudinal anyway; so I see q2-vizard's curveplot as filling the niche for a more general use line/curveplot.

* Edge case: do we always want to enforce that x is numeric? y must always be numeric. But there are cases when a user may want to pass a categorical variable as x, which is then ordered and used for plotting. One example: showing taxonomic classification accuracy (or other metrics) at multiple taxonomic ranks. Or if a user wants to show alpha diversity (or some other metric) at, say, multiple ordered sites that have categorical variables (e.g., named sites that follow a transect). Obviously that user could just make a numeric encoding, plot, and then relabel. But we should consider either:

allowing categorical values to be used for x, which are then ordered alphabetically (or perhaps based on some instructions that the user can pass)
allow relabeling (messy!)

Either way, this will take more thought and is an ENH issue for follow up, not something to worry about in the initial version. In the initial version I think curveplot should require numeric values for x.

Hey @nbokulich,
Thanks for such a detailed follow-up! I discussed all of this with @ebolyen this morning, and here's what we came up with (for both V1 and V2 versions):
V1.

X measure is fixed and numeric
Y measure is a function of X, and can be toggled w/a drop down using any numeric metadata columns
A third measure (we've tentatively called this subject, but could call it group for a more generic name) dictates line faceting and is fixed. Default for this would be false (assuming a single line), and if true, the measure must be a discrete/categorical metadata column, and would also be fixed.
legend/coloring would be utilized for a 4th measure - the default coloring would be based on the subject/group measure, but any other chosen categorical metadata column could be selected from a drop down
Replicates of X won't be allowed in V1
This version can replace the line plot in RESCRIPt

V2.

X measure is fixed & ordered (vs. numeric) to allow for certain categorical values such as taxonomy/etc to be utilized
Replicates of X are allowed and a param is implemented to select replicate handling (i.e. how to model replicates with the line - either via mean/mode/median, or min/max connection)

Here's a sketch I made to help with the visual representation of what V1 will look like:

Let me know if you have any thoughts/concerns about anything above!

Hi @lizgehret sounds like a good plan!
Though replicate handling will be important to add soon, as I expect most use cases to have replicates so erroring out will block use until that is added.

Notes for myself:

faceting works both with/without multiple groups
curve type works for linear & step
[ ] y dropdown 'works' but marks still need to update based on the selected y_column
[ ] Colorby dropdown 'works' but mark coloring still needs to update based on the selection
[ ] legend needs to be fixed

Okay this should be ready for a first round of review - @gregcaporaso @nbokulich feel free to play around with this with a few different datasets. I've been using a modified version of PD mice, so any changes that are needed may arise from trying to break things with bigger/different datasets.

Something to note is that I refactored the existing tests into a few that test the helper method for measure validation (used heavily for both scatterplot and curveplot) which removes a lot of duplicated tests. This isn't technically within the scope of this new visualizer, but it was helpful to do this here to speed up testing and to clean up tests within curveplot. Apologies in advance for adding to the diff with those changes 🙇🏼♀

Hi @lizgehret looks good to me for V1!
I was only able to test with the PD mouse dataset — other/larger datasets I have basically all have X replicates, which adds to my opinion that V2 will be essential before this is ready for action.
I like that line/step options are given! From the illustration above and the name change to curveplot I also assumed that a curve option would be exposed — is this in the planning already? Could be a neat option to add.
The error message about X replicates could be improved if this is a permanent feature, but I think this is only temporary anyway so that's fine.
Note: I only did user testing, I did not review the code.

Thanks for the review @nbokulich! A few updates below:

I was only able to test with the PD mouse dataset — other/larger datasets I have basically all have X replicates, which adds to my opinion that V2 will be essential before this is ready for action.

Okay so I spent quite a bit of time discussing this with @gregcaporaso and @cherman2 offline, and here's what we've decided is the best approach. It's important that we are very clear in any extrapolation we are making on a user's data (i.e. creating lines that aren't a direct connection between two existing data points, but instead some average of that particular set of data points) while also providing enough flexibility in the allowable inputs so that users aren't tempted to modify their metadata to work with this visualization (thus inherently masking the true nature of their data).
X and Y will both be fixed numerical measures in this update, and a user must specify explicitly whether or not the measure they're choosing for X will contain replicates. If replicates are enabled, they also must specify the type of extrapolation that will be made on their data (mean, median, mode). They can also select a discrete measure for 'facetBy' (this name replaces 'group' in the current draft). Y is a fixed measure in this update because there is now a requisite pre-processing step that occurs prior to the data being passed into the Vega spec - all Y values associated with the replicate X value(s) are averaged and then passed into the vega spec as a separate table, which will be used to draw an 'average' line that will display where the average at each set of points falls with respect to the actual data (which will also be plotted as circular marks). I'm also going to add a tooltip to hover over each 'point' (both actual XY points and averaged 'point' locations) that will display the coordinates along with the extrapolation method (if replicates are enabled).

I like that line/step options are given! From the illustration above and the name change to curveplot I also assumed that a curve option would be exposed — is this in the planning already? Could be a neat option to add.

Good question - the name change was something @ebolyen had suggested since we're technically creating mathematical curves to connect data points. But I think it's going to cause more confusion for folks since this won't include a modeled 'best fit' curve - which I think is what most users associate a curve with. I'm going to revert the name back to lineplot!

Thanks @lizgehret!

If replicates are enabled, they also must specify the type of extrapolation that will be
made on their data (mean, median, mode)

I suggest making the default median in this case, since that's the most sensible generally but some users might choose mean by default if not provided with a default value.
I also don't think mode is needed and suggest not providing it as an option. I generally find that to not be very useful for continuous values.

Okay, updated version should be ready for another round of review (cc @gregcaporaso). I created a little dummy dataset with columns that I used for examples with/without replicates and faceting. Here's what each case looks like:

Note that I took these two screenshots (no faceting mean/median) prior to adding the subtitle that contains the average method.

There is also a tooltip hover for the actual data points that contains all of the metadata. Let me know if there's any other functionality that should be added for Oct release (tests still need to be added).

Another note @gregcaporaso is that the way the inputs are configured, there won't be a default for 'average' since it has to be optional (for cases where replicates will be set to False). So users just have to explicitly set either 'median' or 'mean' for their average if they've enabled replicates.

@lizgehret there's 3 smallish merge conflicts it looks like


personal blog template added.

Description
(Describe your project over here.)
Checklist

[x] I've been assigned an issue related to this PR.
[ ] I've used beautifiers.
[x] I've added my Project's name and description to Index.md
[x] I've made a README.md file for my Project.
[x] The README.md file of my projrct contains Project title, Description, Use of project, Set up, Stack used and Output (Screenshots).

Related Issues or Pull Requests number
#285

hey ! Please have a look @harshita2216

Will you tell me what is this code factor. I am curious
On Sat, Jul 24, 2021, 21:43 S Harshita @.***> wrote:

@.**** requested changes on this pull request.
https://www.codefactor.io/repository/github/ayushparikh-code/web-dev-mini-projects/pull/288
Kindly make the following changes.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/Ayushparikh-code/Web-dev-mini-projects/pull/288#pullrequestreview-714244849,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ARVI6WOGM7R4TAUOISQJWRLTZLRDFANCNFSM5A4MRRCA
.

i changed all the code as mentioned in the issues but still the changes are not reflected. @harshita2216

I am closing this PR as one more PR is created by mistake .


💄: more file browser tweaks

With the previous folder visible as '..' we can drop files there too so it's easier to move files around.

A bit less off.

Show checkboxes on hover on bigger screens (> 60em):

Always show on smaller screens:

You can now drop files to 'Add to IPFS' button so you've got a little more dropping area haha


Import a dashboard doesn't refresh the dashboard list

to repro, try to import a dashboard. The import is successful but the screen doesn't update until you refresh.

This no longer seems to be a problem


Race condition when creating new job from client

When creating several new jobs in parallel the client.innerhandler["c"] will be overwritten in client/client.go:225 which down the line leads to jobs getting results of the wrong job.
Proposed solution is to lock the do function only allowing one job to be set up at a time. Pull request: #75

Fixed in 9735b2e54fa92ae6e45a0935a583f3ef425edcf9


onLoadXX callbacks not working due to native props assignment order

React core assign native props to FFFastImageView using a NSDictionary. So when you do things like this
<FastImage source=xxx onLoad=xxx onLoadError=xxx />
on native side props could assigned in order of
onLoad=xxx
onLoadeError=xxx
source=xxx

or
source=xxx
onLoad=xxx
onLoadeError=xxx

The second order will make the component totally ignores onLoad/onLoadError.
Image load callbacks are code blocks that capture scopes at the moment of setSource, where _onFastImageLoad _onFastImageError are nil because  they are not set yet. So event if they got assigned right after setSource, the image load callback blocks captures nil value so they will be ignored.
I tried changing them to self.onFastImageLoad and self.onFastImageError, it does improve the issue, but doesn't totally fix it.
It seems when the image is cached, image load callbacks get invoked right inside of setImage, so onLoad/onLoadError gets ignored again...

fetch image async can fix this
///FFFastImageView.m
- (void)setSource:(FFFastImageSource *)source {
    if (_source != source) {
        _source = source;

        // add this line
        dispatch_async(dispatch_get_main_queue(), ^{
            // Set headers.
            [_source.headers enumerateKeysAndObjectsUsingBlock:^(NSString *key, NSString* header, BOOL *stop) {
                [[SDWebImageDownloader sharedDownloader] setValue:header forHTTPHeaderField:key];
            }];

            // Set priority.
            SDWebImageOptions options = 0;
            options |= SDWebImageRetryFailed;
            switch (_source.priority) {
                case FFFPriorityLow:
                    options |= SDWebImageLowPriority;
                    break;
                case FFFPriorityNormal:
                    // Priority is normal by default.
                    break;
                case FFFPriorityHigh:
                    options |= SDWebImageHighPriority;
                    break;
            }

            if (_onFastImageLoadStart) {
                _onFastImageLoadStart(@{});
                hasSentOnLoadStart = YES;
            } {
                hasSentOnLoadStart = NO;
            }

            // Load the new source.
            [self sd_setImageWithURL:_source.uri
                    placeholderImage:nil
                             options:options
                            progress:^(NSInteger receivedSize, NSInteger expectedSize, NSURL * _Nullable targetURL) {
                                double progress = MIN(1, MAX(0, (double) receivedSize / (double) expectedSize));
                                if (_onFastImageProgress) {
                                    _onFastImageProgress(@{
                                                           @"loaded": @(receivedSize),
                                                           @"total": @(expectedSize)
                                                           });
                                }
                            } completed:^(UIImage * _Nullable image,
                                          NSError * _Nullable error,
                                          SDImageCacheType cacheType,
                                          NSURL * _Nullable imageURL) {
                                if (error) {
                                    if (_onFastImageError) {
                                        _onFastImageError(@{});
                                        if (_onFastImageLoadEnd) {
                                            _onFastImageLoadEnd(@{});
                                        }
                                    }
                                } else {
                                    if (_onFastImageLoad) {
                                        _onFastImageLoad(@{});
                                        if (_onFastImageLoadEnd) {
                                            _onFastImageLoadEnd(@{});
                                        }
                                    }
                                }
                            }];
        });
    }
}

image unload -> SDWebImage load image async
image in cache -> SDWebImage load image sync
This issue because of setSource and fetch image done before  setOnFastImageError&set onFastImageLoad

Yeah it's sort of a hassle to manage these props when they come in at different times. Should be resolved now. See: https://github.com/DylanVann/react-native-fast-image/pull/74


Fork of public-apis

As there are so many Pull Requests needed to be reviewed but has remained neglected even if they do they cannot be merged due to the explicit rights of the owner of the organization. I had made a fork of this repository which you can find here free-public-apis
Interested volunteer can mail or PR to the repo. free-public-apis/free-public-apis
NOTE: IF YOU HAVE ANY PROBLEM REGARDING THE ORGANIZATION OR REPOSITORY MENTION ABOVE FREE FEEL TO CONTACT ME.

@PawelBorkar Do you think we still need to fork this repo?

@PawelBorkar Do you think we still need to fork this repo?

No.


Precision changes that modify numeric values in V*SliderEdit do not emit a ValueChanged signal

The V*SliderEdits need a new signal for when a change to their format changes their held value as well.
Something like FormatAndValueChanged().
To reproduce:

Open any dataset and create a Volume Renderer
In the Appearance tab, type .2001 into the Ambient lighting slider-edit
Right click the slider-edit, and reduce the decimal precision to 3, modifying the new value to .200.  A FormatChanged() signal has been emitted, but not a ValueChanged() signal, which would propagate this new value up to Params.
Increase the precision to 4, and .2000 is displayed.
Move the scene, and .2001 will be restored

Question regarding the behavior: when you change both the precision and the value, for example, change it from 0.200 to 0.3005, is a ValueChanged signal emitted?

@shaomeng, I believe your scenario is a 2 step process, which would emit two signals.
First the FormatChanged() signal would be emitted to bring the decimal precision from 3 to 4.  Then the ValueChanged() signal would be emitted when the user changes from 0.2000 to 0.3005.

@shaomeng, I believe your scenario is a 2 step process, which would emit two signals.
First the FormatChanged() signal would be emitted to bring the decimal precision from 3 to 4. Then the ValueChanged() signal would be emitted when the user changes from 0.2000 to 0.3005.

I believe what you described should be the behavior the widget targets for. But the point I wanted to make is, if the widget fails to emit a "ValueChanged" signal when a user changes it from 0.200 to 0.3005, then it's a more severe bug than "medium."


Fail compilation on targets w/o soundness proof

TODO: Figure out how to make sure we block the next minor version release on adding back all of the targets that people actually rely on.
Makes progress on #383

Given the discussion in https://github.com/rust-lang/unsafe-code-guidelines/issues/461, I'm going to close this on the assumption that we'll adopt a policy of not reasoning about target architecture. We can always re-open if that's not how it plays out.


cache_promote plugin: put a mutex-protected critical section into a Continuation.

This is a draft PR that takes the place of https://github.com/apache/trafficserver/pull/5406 .

[approve ci autest]

[autest ci Clang-Analyzer]

[approve ci Clang-Analyzer]

[approve ci autest]

@bryancall  vetoed https://github.com/apache/trafficserver/pull/4848 , which this PR depends on, so this PR thus effectively vetoed also.

@ywkaras I didn't veto it.  The procedure is to close old PRs that haven't had any recent activity. Please read the comment at the end of the PR.

I have no way of forcing meaningful activity so thus it is effectively vetoed.


dropdown issue

Hi, i have issue in drop down  after selecting any status it should show only select option  in a same page please help me to close the issue
once i select an option its showing option values but i need only  select option  default value.
my code :

Select
{{n.name}}

	</div>

Hi, i have issue in drop down after selecting any status it should show only select option in a same page please help me to close the issue
once i select an option its showing option values but i need only select option default value.
my code :+1:
<select id="defaultSelected" class="browser-default" name= "" (change)="bulkActionStatus($event.target.value)"(disabled)="disabled">
<option selected="defaultSelected" [value]="">Select
<option *ngFor = "let n of listStockStatus" [value] = "n.id" >{{n.name}}


new weapon: laser

Laser component.
-Will be a raycast with reflection off objects.
-Asteroid heats up with laser exposure.
-Laser cuts polygon at intersection.
-Yields more loot than destructive cannons.
-player can cycle through current equipped weapon: cannon, chargeCannon, laser

http://www.iforce2d.net/b2dtut/raycasting

https://asawicki.info/news_1301_reflect_and_refract_functions.html

https://phet.colorado.edu/sims/html/bending-light/latest/bending-light_en.html

ohh pretty colors
https://www.youtube.com/watch?v=-VjbYI-eaTQ
https://www.youtube.com/watch?v=nVjGzmTURUk

Slice: we get the entry point from the first raycast, but how do we get the exit point for the laser?
Answer: perform another raycast! but flip the start and end point, so the ray is cast from the end point of the laser towards the caster. eg:
world.rayCast(callback,p1,p2); //entry point
world.rayCast(callback,p2,p1); //exit point
https://www.emanueleferonato.com/2011/06/25/slicing-splitting-and-cutting-objects-with-box2d-part-2/

another simulator
https://phydemo.app/ray-optics/simulator/
https://github.com/ricktu288/ray-optics


pkg/report, pkg/repro: fix report parsing

pkg/repro expects pkg/report to return CrashStart index in the whole report, not only in the kernel output.

We badly need reproducers, we are getting dozens of bugs without reproducers due to this. So I merged it. Please add a test in a subsequent commit.


Skip cluster tests until fixed

@dgnorton @corylanou

Don't the meta tests need to be skipped too?

Yeah, I was wondering about those.

TestCluster_OpenRaft is another culprit which would fall in the meta test category, so yeah, skipping those might make sense.

(I think OpenRaft has been issue, not fully sure).

I would be nervous about skipping those just yet @pauldix -- they are pretty important and the failure rate after this merge may be back to tolerable.
OK if I merge this skip, and we deal with those separately?

In other words I'd rather deal with a tolerably low-failure rate (which we don't have right now) in the meta package if if prevents us from checking in a real bug in that code.

We may need a different approach to these tests.  I'll discuss it in todays meeting.  For now, +1 to get the tests stable until we can re-address.


wrong cloning

with this https://github.com/crystaluniverse/publishingtools/blob/development/deployment/threefoldwebsitesandwikis.toml i find
code_production for /code_production/github/threefoldfoundation/info_foundation# always development while it should be master

should be done already @abom please link to the right commit resolving this issue and close

Fixed in crystaltools and the added shard.lock to reference latest version.


PdfException: Failed to load PDF document (FPDF_GetLastError=3)

When I load this url an exception occurs.
PdfViewer.uri(
    Uri.parse("https://www.tutorialspoint.com/flutter/flutter_tutorial.pdf"),
    ...
)

Testing environment: Flutter MacOS

But when I try with another url it works fine.
Ex: https://www.diva-portal.org/smash/get/diva2:1560848/FULLTEXT01.pdf

Your report does not contain any info. about what exception thrown actually. Normaly you should provide more specific info. for your issue.
Anyway, just testing the PDF with my environment results in
PdfException: Failed to load PDF document (FPDF_GetLastError=3).

...

And the error 3 means FPDF_ERR_FORMAT. I don't know what it means because it's a pdfium error. But opening the PDF with Google Chrome 127.0.6510.4 succeeded. So it seems to be some kind of pdfium's known issue with the pdfium version pdfex uses.

https://github.com/bblanchon/pdfium-binaries/releases shows me that the latest pdfium binary for non-darwin platforms is 6517.
OK, it may be the time to upgrade the modules...

Futher investigation shows me that the downloading of the file failed constantly.

Mmm, the server, named ECLF (I'm not sure what it is), responses with HTTP 400...

And, I found that my code does not have correct HTTP error handling code (that is not the cause of the issue anyway).

@espresso3389 I tried using InternetFile.get it returns 200
`dart
import 'dart:async';
import 'dart:typed_data';
import 'package:flutter/material.dart';
import 'package:http/http.dart' as http;
typedef InternetFileProgress = void Function(
int receivedLength,
int contentLength,
);
class InternetFile {
static Future get(
String url, {
Map<String, String>? headers,
InternetFileProgress? progress,
bool force = false,
String method = 'GET',
bool debug = false,
}) async {
final completer = Completer();
final httpClient = http.Client();
final request = http.Request(method, Uri.parse(url));
if (headers != null) {
request.headers.addAll(headers);
}
final response = httpClient.send(request);
List<int> bytesList = [];
int receivedLength = 0;

response.asStream().listen((http.StreamedResponse request) {
  request.stream.listen(
    (List<int> chunk) {
      receivedLength += chunk.length;
      final contentLength = request.contentLength ?? receivedLength;
      final percentage = receivedLength / contentLength * 100;

      if (debug) {
        debugPrint('download progress: $receivedLength of '
            '$contentLength ($percentage%)');
      }
      progress?.call(receivedLength, contentLength);

      bytesList.addAll(chunk);
    },
    onDone: () {
      final bytes = Uint8List.fromList(bytesList);

      completer.complete(bytes);
    },
    onError: completer.completeError,
  );
}, onError: completer.completeError);
return completer.future;

}
}
`

Yes, I think the actual cause of the issue is the server's implementation (it mis-handles HTTP range requests) but I'm now working on my code to fix it.

1.0.66 fixes the issue.

@espresso3389  Thank you so much for quick fix!

@espresso3389 The above error has been fixed.
However, there is still a previous error.
When I try with this url: https://www.kansaigaidai.ac.jp/asp/img/pdf/82/7a79c35f7ce0704dec63be82440c8182.pdf
then an exception occurs: LateInitializationError: Field '_cacheBlockCount@1436474497' has not been initialized.

I see. It's just another issue. OK, I'll check it.


BasicTextField selection collapses to a zero span at the cursor location when focus is lost

Here is the sample to reproduce this on 1.2.0 and 1.3.0-rc01:
import androidx.compose.foundation.layout.*
import androidx.compose.foundation.text.BasicTextField
import androidx.compose.material.Button
import androidx.compose.material.Text
import androidx.compose.runtime.*
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.text.TextRange
import androidx.compose.ui.text.input.TextFieldValue
import androidx.compose.ui.unit.DpSize
import androidx.compose.ui.unit.ExperimentalUnitApi
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.*

@ExperimentalUnitApi
fun main() = application {
    val state = rememberWindowState(
        placement = WindowPlacement.Floating,
        position = WindowPosition.Aligned(Alignment.Center),
        size = DpSize(600.dp, 400.dp)
    )

    Window(
        title = "Demo",
        state = state,
        onCloseRequest = ::exitApplication
    ) {
        SampleStyles()
    }
}

@ExperimentalUnitApi
@Composable
fun SampleStyles() {
    var textSelectionRange by remember { mutableStateOf(TextRange.Zero) }
    println("Range $textSelectionRange")

    Column(modifier = Modifier.fillMaxSize()) {
        Row(modifier = Modifier.fillMaxWidth()) {
            Button(onClick = {}) {
                Text("Sample")
            }
        }
        BasicTextField(
            value = TextFieldValue(
                text = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor",
                selection = textSelectionRange
            ),
            onValueChange = {
                println("Changing range to ${it.selection}")
                textSelectionRange = it.selection
            },
            modifier = Modifier.fillMaxWidth().fillMaxHeight(fraction = 1.0f)
        )
    }
}

Run the app. Select a range in the text - it will print the selected range in the console. Now click the button at the top of the window and look at the updated range printed in the console.
https://github.com/JetBrains/androidx/commit/ca869b1504e96e82224a11596b823fbe7dc2fc82#diff-42592036468eac046f04d9f25ac21f134ed22d3b55e602da3c56bb0c9f206a7c may be the change that caused this behavior change. Note that version 1.1.0 and the Android variant do not lose the selection on focus loss.
In Swing, by the way, selection is not lost on focus loss. It is not drawn by default, and there is a workaround to keep the selection visible:
            textPane.setCaret(new DefaultCaret() {
                @Override
                public void setSelectionVisible(boolean visible) {
                    super.setSelectionVisible(true);
                }
            });

The target use case is a simple text editor that allows toggling bold, italic, underline, strikethrough, font size, font family on the selected span. Think a text pane with a strip of buttons on the side to initiate those actions.

To be precise, this is the change that causing such a behaviour:
https://github.com/JetBrains/androidx/commit/ca869b1504e96e82224a11596b823fbe7dc2fc82#diff-42592036468eac046f04d9f25ac21f134ed22d3b55e602da3c56bb0c9f206a7cL563
- .focusableInNonTouchMode(enabled = enabled, interactionSource = interactionSource)
+ .focusable(enabled = enabled, interactionSource = interactionSource)

It was intentional, but apparently it didn't consider the use case like you described. Thank you! We'll need to come up with some better logic.
cc: @igordmn

I checked this code, changing the focus programmatically, not pressing the button:
import androidx.compose.foundation.focusable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.fillMaxHeight
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.text.BasicTextField
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.focus.FocusRequester
import androidx.compose.ui.focus.focusRequester
import androidx.compose.ui.text.TextRange
import androidx.compose.ui.text.input.TextFieldValue
import androidx.compose.ui.window.Window
import androidx.compose.ui.window.application
import kotlinx.coroutines.delay

fun main() = application {
    Window(onCloseRequest = ::exitApplication) {
        var textSelectionRange by remember { mutableStateOf(TextRange.Zero) }
        println("Range $textSelectionRange")

        val focusRequester = remember { FocusRequester() }
        Box(Modifier.focusRequester(focusRequester).focusable())

        BasicTextField(
            value = TextFieldValue(
                text = "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor",
                selection = textSelectionRange
            ),
            onValueChange = {
                println("Changing range to ${it.selection}")
                textSelectionRange = it.selection
            },
            modifier = Modifier.fillMaxWidth().fillMaxHeight(fraction = 1.0f)
        )

        LaunchedEffect(Unit) {
            delay(2000)
            focusRequester.requestFocus()
        }
    }
}

In Compose 1.1.1 and Compose 1.2.2 it works the same - we lose selection in the text field.
So it is not a regression. In Compose 1.2.0 clickable there just new feature on desktop - clickable components receive focus on click.
It also not a bug, because it programmed intentionally to clear selection when we lose focus.
Maybe it is not right to clear selection from UX perspective or platform guidelines perspective, but we need to investigate that. If we decided to not clear selection - we probably should change the color of the selection (usually it is gray). Also it is not obvious how will 2 text fields behave. Should they have its own selection? Maybe we need to introduce something like focus groups, that can have their own independent focus inside the group.

The target use case is a simple text editor that allows toggling bold, italic, underline, strikethrough, font size, font family on the selected span

It looks like those are icon buttons in a toolbar, and we need to not switch focus to them, when we click on them 🤔. I.e. they shouldn't be focusable. @eymar, I think, we either need to rethink our approach to make all clickable's focusable (make only some widgets), or provide a way to disable focusability.

provide a way to disable focusability

One of the current workarounds - don't use clickable, or IconButton, use Box + onClick

From the UX perspective, I don't think it would be the right move to disable focus on such toolbar buttons or comboboxes. I'd still want them to be fully traversible.
I would rather prefer to explicitly mark the text field (read only or not) to not lose selection when it loses focus.

As an example, see IntelliJ. You can select a block in the editor tab, then start Search / Search & Replace and use tab to traverse the text fields and buttons in that bar without losing the selection in the editor.

One more report for this issue: https://github.com/JetBrains/compose-jb/issues/2687

It's been a year since this issue has been reported. Any progress ?
I can confirm this issue is still present in Compose 1.5.11.
This is a real problem for desktop apps that manipulate text inside text fields.
Is there a workaround other than banishing clickable and IconButton from our code (which is painful and only works for buttons, not checkboxes for example) ?

Waiting for a fix, I found the folowing workaround.
I haven't found any undesirable side effect, but that doesn't mean there is none 😅.
var value: TextFieldValue by remember { mutableStateOf(TextFieldValue()) }
var previousSelection by remember { mutableStateOf(TextRange(0)) }
BasicTextField(
    value = value,
    onValueChange = {
        previousSelection = value
        value = it
    },
    modifier = Modifier
        .onFocusChanged {
            if (!previousSelection.collapsed) value = value.copy(selection = previousSelection)
        }
)

Please check the following ticket on YouTrack for follow-ups to this issue. GitHub issues will be closed in the coming weeks.


Design plugin architecture

Brainstorming my thoughts on architecture.
Class Bot will use an IrcClient interface (phpoxford/spires-irc) to connect to Freenode and join the channel. The Bot will need a loop (assuming non-blocking) which receives messages from IRC (messages being chat messages, join notices, quit notices, etc).
Bot will need a MessageParser, which will need an EventDispatcher. The parser will parse the messages and trigger events as required (for example a quit notice could trigger event called UserQuitIrc).
Plugins should listen for certain events and have a method to handle the given event Pluginnable::handleEvent(?).
Plugin event handlers should would in turn need to emit a Result event which the Bot class can consume. This way multiple plugins could handle one command and the bot could process each result event separately.
Thoughts? Too complex? Not complex enough? Idiotic?

I will describe the architecture we chose to you tomorrow, or sooner if you prefer :-)


Added drawing classifier annotate

added drawing classifier annotate

@abhishekpratapa Does this need to go into 5.6?


OnBackgroundHandler is not working when app is terminated

in OnBackgroundHandler callback not recevied when app is start then notification came please try to solve this issue

Duplicate of #1590


Display all subcommands and pruned code

/claim #113
A command with tree structure
parent
    └── child1
        ├── child2
        └── child3

would have the following helpDoc:
COMMANDS

  -  child1         help1
  -  child1 child2  help2
  -  child1 child3  help3

Also, I have deleted some unnecesary self in Args, Options and Commands.

@pablf Can you show before / after what this looks like in a couple examples?


Streamline trees' nodes hierarchy

This PR removes the repeated usage of multiple inheritances to handle active and inactive learning nodes.
We now see the concepts of "active" and "inactive" as states nodes might assume through time, instead of explicitly treating them as different node classes.
The benefits of it are clear most of the time: fewer lines of code to maintain, a simpler structure to understand. Future Cython implementations of the current tree module's members will also benefit from a simplified class structure.
Unfortunately, there is no free lunch. The ARF-based trees and EFDT modify the standard behavior of the learning nodes. As a result, I had to duplicate some code to ensure proper functionality. In all the cases, I preferred to repeat what these "fancy" decision tree algorithms modify in the vanilla Hoeffding Tree rather than reimplementing the learning logic of the nodes they inherit from. In the end, this choice might consume more lines of code, but I believe this is a safer option in the long run. If we fix/update something in the learning nodes of
HoeffdingTree{Classifier/Regressor}, automatically ARFHoeffdingTree{Classifier/Regressor}  and EFDT will reflect those changes.
I cannot say I totally like that, to be fair. I would like not to repeat code at all. So, I am totally open to suggestions on how to solve this puzzle.
Observation: I changed the placement of the learning_one method in HoeffdingTreeClassifier just for aesthetic reasons. It made the git diff tracker crazy :P

During the refactoring, we removed the sortedcontainers. In the case of best_split_suggestions  now we get an unsorted list. We can use a heap (see heapq) to avoid the overhead from sorting.
https://github.com/creme-ml/creme/blob/554e4c542fd085fb0fee1d3f0e0f0a57f71be71a/river/tree/_nodes/base.py#L384-L412

During the refactoring, we removed the sortedcontainers. In the case of best_split_suggestions now we get an unsorted list. We can use a heap (see heapq) to avoid the overhead from sorting.

Just to be sure. Were lines such as the following previously redundant?
https://github.com/creme-ml/creme/blob/63120d370f01da0f44a45090b9b2ea55415ccbff/river/tree/hoeffding_tree_classifier.py#L287
I can use the bisect package to keep the splot suggestions ordered and alleviate the costs of sorting. What do you think?

Each insertion in a list already ordered takes O(log n), thus O(n log n) for inserting n elements. Python's TimSort also has a cost of O(n log n).
Therefore, I think we are in good shape since the trees call sort in the split suggestions.

Each insertion in a list already ordered takes O(log n), thus O(n log n) for inserting n elements. Python's TimSort also has a cost of O(n log n).
Therefore, I think we are in good shape since the trees call sort in the split suggestions.

Thanks for the follow up on this.


MergedBots port

Sorry, I didn't realise I was creating a pull request into the original repo (it's a bit early for that)


Publish to bower repo

Regarding #3119
Added a sub-task to publish.sh that publishes to a specified bower repo.
With this in place would still need a few things for ol3 to be published to bower:

Create openlayers/bower-ol3 with a bower.json similar to that seen at malaretv/bower-ol3
Register package with bower using bower register (see http://bower.io/docs/creating-packages/#register)

Note that this currently depends on bower being install globally via npm. If that is not acceptable bower could be added as dev dependency or the bower version task can be done manually by updating the json and adding the correct tag.

Nice, thanks for getting this started @malaretv! Can't we add a bower.json directly to the ol3 repo, like we do with package.json?

My thinking was to avoid pulling down files unnecessary for production, but now I realize the bower.json provides an ignore field for that purpose. Will revise and remove the need for a separate repo.

yeah but ignore only filters after they have been pulled in I think

@bartvde ah yeah couldn't find any documentation on how the ignore works but looking at the source looks like you're right.
Also I take back what I said above it seems a separate repo actually does make sense here. Bower does not run any postinstall steps so it expects the built js to be in the repo. An alternative to keeping the bower distribution in it's own repository is include the dist/ folder in the tagged releases. This would also eliminate the need to run the closure compiler as post install step for npm.

I think adding the dist/folder to tagged releases makes sense. Many other projects do that too.

OpenLayer 3 is an important JavaScript library that is not in Bower package.

Can I convince anybody here just to add openlayers to their package.json?  If you've got Bower, you've already got npm.
I hope we can convince people not to use the full build of the library (the one published to the npm registry).  Publishing the same full build to the Bower registry will make this even harder.  If we agree on a useful set of custom profiles to publish, then I'd be more open to publishing them on additional registries, but I'm a bit dubious about being able to generate profiles that will satisfy a large enough group of users to be useful.
I want to make ol3 accessible to more users, but I'm not so hot on the idea of people thinking the full build is the right way to go.

I understand and share @tschaub's concerns.
We need some good and official documentation describing how to download/install OpenLayers with node/npm and create a custom build. I think it would help if the OpenLayers node package included a build configuration file that people could use as a base.

I've done some work for an How To about using Browserify and WebPack reusing my knowledge and feedback from #3162
We maybe can adapt it.  The repo for this purpose is https://github.com/webgeodatavore/demo_browserify_webpack_ol3
I didn't work on creating documentation on custom build as there are infos in https://github.com/openlayers/ol3/blob/master/tasks/readme.md but for most end users @elemoine, your viewpoint about custom build is valid IMO.

Closing this as some of the OpenLayers 3 developers disagree with publishing OpenLayers 3 to bower.io. Please reopen if you think this is still worth discussing.


Proposal to make three pull requests

In order to make RTBkit work correctly with Smaato and our needs for statistics, I have had to make three modifications and I would like to discuss them before I make the pull requests.

I needed to make regionCode a Unicode character in bid_request.cc/h. Also region in openrtb.h was changed to Unicode and a test for Appnexus to match this. The reason is that Smaato occasionally sends bid requests with unicode characters in the region field.

The matchedLoss calculation was wrong in post_auction_service.cc

Finally, there is a set of changes in Smaato exchange connector to i) deal correctly with video ads in a similar manner to AppNexus exchange connector, ii) add creative attributes, iurl and crid

Details of modifications found below:
The following modification  changed the type of a regionCode to Unicode. It was needed because bid requests from Smaato might have unicode characters in them.
Changes to bid_request.cc/h

diff --git a/rtbkit/common/bid_request.h b/rtbkit/common/bid_request.h
index 0309c7a..4b77dbd 100644
--- a/rtbkit/common/bid_request.h
+++ b/rtbkit/common/bid_request.h
@@ -234,7 +234,7 @@ struct Location {
     }

     std::string countryCode;
-    std::string regionCode;
+    Datacratic::UnicodeString regionCode;
     Datacratic::UnicodeString cityName;
     Datacratic::UnicodeString postalCode;

diff --git a/rtbkit/common/bid_request.cc b/rtbkit/common/bid_request.cc
index f909c44..d1eb691 100644
--- a/rtbkit/common/bid_request.cc
+++ b/rtbkit/common/bid_request.cc
@@ -426,7 +426,10 @@ Datacratic::UnicodeString
 Location::
 fullLocationString() const
 {
-    Datacratic::UnicodeString result(countryCode + ":" + regionCode + ":") ;
+    Datacratic::UnicodeString result(countryCode) ;
+    result += ":";
+    result += regionCode;
+    result += ":";
     result += cityName ;
     result += ":";
     result += postalCode;

diff --git a/rtbkit/openrtb/openrtb.h b/rtbkit/openrtb/openrtb.h
index 6e69dfb..05c2eab 100644
--- a/rtbkit/openrtb/openrtb.h
+++ b/rtbkit/openrtb/openrtb.h
@@ -916,7 +916,7 @@ struct Geo {
     Datacratic::TaggedDouble lat;        ///< Latitude of user (-90 to 90; South negative)
     Datacratic::TaggedDouble lon;        ///< Longtitude (-180 to 180; west is negative)
     std::string country;         ///< Country code (ISO 3166-1 Alpha-3)
-    std::string region;          ///< Region code (ISO 3166-2)
+    Datacratic::UnicodeString region;          ///< Region code (ISO 3166-2)
     std::string regionfips104;   ///< Region using FIPS 10-4
     std::string metro;           ///< Metropolitan region (Google Metro code)
     Datacratic::UnicodeString city;        ///< City name (UN Code for Trade and Transport Loc)

diff --git a/rtbkit/plugins/bid_request/testing/appnexus_bid_request_test.cc b/rtbkit/plugins/bid_request/testing/appnex
index a760e28..0675bda 100644
--- a/rtbkit/plugins/bid_request/testing/appnexus_bid_request_test.cc
+++ b/rtbkit/plugins/bid_request/testing/appnexus_bid_request_test.cc
@@ -161,7 +161,7 @@ BOOST_AUTO_TEST_CASE( test_openrtb_from_appnexus )
     BOOST_CHECK_EQUAL(ortbReq->device->make, Datacratic::UnicodeString("1001"));
     BOOST_CHECK_EQUAL(ortbReq->device->model, Datacratic::UnicodeString("10001"));
     BOOST_CHECK_EQUAL(ortbReq->device->geo->country, "US");
-    BOOST_CHECK_EQUAL(ortbReq->device->geo->region, "NY");
+    BOOST_CHECK_EQUAL(ortbReq->device->geo->region, Datacratic::UnicodeString("NY"));
     BOOST_CHECK_EQUAL(ortbReq->device->geo->city, Datacratic::UnicodeString("New York"));
     BOOST_CHECK_EQUAL(ortbReq->device->geo->zip, Datacratic::UnicodeString("10014"));
     BOOST_CHECK_EQUAL(ortbReq->device->geo->dma, "501");

The following modification fixes an error with the statistics of MatchedLosses.

diff --git a/rtbkit/core/post_auction/post_auction_service.cc b/rtbkit/core/post_auction/post_auction_service.cc
index d2136ff..800f860 100644
--- a/rtbkit/core/post_auction/post_auction_service.cc
+++ b/rtbkit/core/post_auction/post_auction_service.cc
@@ -545,9 +545,11 @@ doMatchedWinLoss(std::shared_ptr<MatchedWinLoss> event)
     if (event->type == MatchedWinLoss::Win || event->type == MatchedWinLoss::LateWin) {
         lastWinLoss = Date::now();
         stats.matchedWins++;
-    } else {
+    } else if (event->type == MatchedWinLoss::Loss) {
         stats.matchedLosses++;
-        return;
+       //   return;
+    } else {
+      return;
     }

     if (analytics) event->publish(*analytics);

The following modification handles an error in the Smaato exchange connector that results from bid requests for video ads being sent from time to time. An additional fix was to add support for image url (iurl), creative id (crid) and creative attributes.

diff --git a/rtbkit/plugins/exchange/smaato_exchange_connector.cc b/rtbkit/plugins/exchange/smaato_exchange_connector.cc
index 8110743..d95c048 100644
--- a/rtbkit/plugins/exchange/smaato_exchange_connector.cc
+++ b/rtbkit/plugins/exchange/smaato_exchange_connector.cc
@@ -228,6 +228,9 @@ ExchangeConnector::ExchangeCompatibility
     getAttr(result, pconf, "adid", crinfo->adid, includeReasons);
     getAttr(result, pconf, "adomain", crinfo->adomain, includeReasons);
     getAttr(result, pconf, "mimeTypes", crinfo->mimeTypes, includeReasons);
+    getAttr(result, pconf, "cat", crinfo->cat, includeReasons);
+    getAttr(result, pconf, "crid", crinfo->crid, includeReasons);
+    getAttr(result, pconf, "attr", crinfo->attr, includeReasons);
     result.info = crinfo;

     return result;
@@ -240,21 +243,32 @@ ExchangeConnector::ExchangeCompatibility
             const void * info) const {
       const auto crinfo = reinterpret_cast<const CreativeInfo*>(info);

-      // now go through the spots.
-      for (const auto& spot: request.imp) {
-          //const auto& mime_types = spot.banner->mimes;
-       for (const auto& mimeType : spot.banner->mimes) {
-              if (std::find(crinfo->mimeTypes.begin(), crinfo->mimeTypes.end(), mimeType.type)
-                      != crinfo->mimeTypes.end()) {
-                  this->recordHit ("blockedMime");
-                  return true;
-              }
+      // 1) first check for blocked content categories
+      const auto& blocked_categories = request.restrictions.get("blockedCategories");
+      for (const auto& cat: crinfo->cat)
+       if (blocked_categories.contains(cat)) {
+         this->recordHit ("blockedCategory");
+         return false;
        }
-      }
-
-       return false;
+
+      // 2) now go through the spots.
+      // Note: this has to be last as it returns false it does not find a supported mime type
+      for (const auto& spot: request.imp) {
+       // Check that spot.banner is not empty (which it is if the bid request is video)
+       if (spot.banner != nullptr) {
+         for (const auto& mimeType : spot.banner->mimes) {
+           if (std::find(crinfo->mimeTypes.begin(),
+                         crinfo->mimeTypes.end(), mimeType.type)
+               != crinfo->mimeTypes.end()) {
+             return true;
+           } // if
+         } // for (const
+       } // if (spot.banner
+      } // for (const
+      this->recordHit ("blockedMime");
+      return false;
   }
-
+
 void
 SmaatoExchangeConnector::
 setSeatBid(Auction const & auction,
@@ -312,6 +326,9 @@ setSeatBid(Auction const & auction,
     b.adm = crinfo->adm;
     b.adomain = crinfo->adomain;
     b.nurl = crinfo->nurl;
+    b.attr = crinfo->attr;
+    b.crid = crinfo->crid;
+    b.iurl = cpinfo->iurl;
 }

 } // namespace RTBKIT

diff --git a/rtbkit/plugins/exchange/smaato_exchange_connector.h b/rtbkit/plugins/exchange/smaato_exchange_connector.h
index e7b0987..a62e9b0 100644
--- a/rtbkit/plugins/exchange/smaato_exchange_connector.h
+++ b/rtbkit/plugins/exchange/smaato_exchange_connector.h
@@ -50,6 +50,7 @@ struct SmaatoExchangeConnector: public OpenRTBExchangeConnector {

     struct CampaignInfo {
       Id seat;                ///< ID of the exchange seat
+      std::string iurl;       ///< Image URL for content checking
     };

     virtual ExchangeCompatibility
@@ -62,6 +63,9 @@ struct SmaatoExchangeConnector: public OpenRTBExchangeConnector {
       std::string nurl;       ///< Win notice URL
       std::vector<std::string> adomain; ///< Advertiser Domain
       std::vector<std::string> mimeTypes; ///< MIME Types for this creative
+      std::vector<std::string> cat; ///< Categories applicable for this creative
+      Id crid;                                        ///< Creative ID
+      OpenRTB::List<OpenRTB::CreativeAttribute> attr; ///< Creative attributes
     };

     virtual ExchangeCompatibility

diff --git a/rtbkit/plugins/exchange/testing/smaato_exchange_connector_test.cc b/rtbkit/plugins/exchange/testing/smaato_exchange_connector_test.cc
index 0650296..2b451f0 100644
--- a/rtbkit/plugins/exchange/testing/smaato_exchange_connector_test.cc
+++ b/rtbkit/plugins/exchange/testing/smaato_exchange_connector_test.cc
@@ -108,6 +108,8 @@ BOOST_AUTO_TEST_CASE( test_smaato )
         c.providerConfig["smaato"]["adomain"][0] = "rtbkit.org";
         c.providerConfig["smaato"]["crid"] = c.name;
         c.providerConfig["smaato"]["iurl"] = "http://www.gnu.org";
+       c.providerConfig["smaato"]["cat"][0] = "IAB1";
+       c.providerConfig["smaato"]["attr"][0] = 0;
         c.providerConfig["smaato"]["adm"]
             = "<ad xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xsi:noNamespaceSchemaLocation=\"smaato_ad_v0.9.xsd\" modelVersion=\"0.9\"><imageAd><clickUrl>http://mysite.com/landingpages/mypage
            c.providerConfig["smaato"]["nurl"] = "http://mywinurl.com/win?width=" + std::to_string(c.format.width) + "&height="+ std::to_string(c.format.height) + "&auction_price=${AUCTION_PRICE}&auction_id
(END)

No comments to these issues?

I noticed that 2 issue has been addressed by a pull request. Can someone approve this, please?
Part of issue 3 is also addressed by another pull request, but not everything.  Smaato has updated their requirements, so more is needed. They also want to move to OpenRTB 2.4 so that is also needed.


ModelZoo versioning

Overview
The goal is to allow for building older versions of Autoware.Auto that rely on past versions of the models.
Currently the ModelZoo repository is tagged as "X.Y.Z" (1.1.0 at the moment), but without rules on how to tag. New tagging rules on version updates would follow the same numbering scheme with:

X: API changes

code will not compile (or will fail at runtime after loading the TVM libraries dynamically)
e.g. updated descriptor interface
e.g. updated TVM version

Y: behavior changes

changes in either performance or results (or failure to run altogether) when doing the inference
e.g. updated model artifacts

models modified (changed model files, added new models, deleted models, ...)
scripts modified (change in compilation options, change in targeted backends, ...)

Z: convenience changes

Autoware won't care about this
e.g. updated docker image

Currently, only the most recent version of the artifacts is held in the bucket, as "/networks_${ARCH}.tar.gz". It is overwritten each time there is a change to the models in this repository. Part of this design is on how to manage multiple versions to be kept in the bucket.
Tagging would be manually done by the maintainers. They have to assess whether changes to the repository fall into any of the previous categories. The reason for not automating the process is to allow for intelligent control of the versioning which in turn minimizes the amount of artifacts uploaded to the bucket.
This version can be reflected in the name of the artifacts held in the AWS S3 bucket, and a specific version can be targeted by the Autoware codebase. When new features are needed by Autoware, the targeted version can be updated in a commit, together with testing that upgrading the version doesn't break packages using inference from the provided networks.
Items
S3 bucket

ACTION: check by how much the amount of storage required by the artifacts can be increased
To save space, the artifacts in the bucket could be cleaned up at different points in time:

when the artifact version targeted by Autoware changes, the artifact versions that were skipped between the old and new targets can be deleted
potentially, on an Autoware release, the artifacts that are not used in previous releases could be cleaned up from the bucket, saving space (but breaking previous non-tagged versions of Autoware that target those deleted artifacts)

Model Descriptor (MZ/AA interface)

Add a "version" field to the structure containing the same X.Y.Z information

adding this field will trigger an update to the X version number according to the tagging rules (ModelZoo 2.0.0)
allows the code to target a range of X and Y versions, and display a useful warning or error message when the model's version doesn't match (error on known unsupported versions and warning on unknown versions)

ModelZoo

On pushes to the "master" branch that modify models or scripts:

continue uploading and overwriting artifacts; but to "/bleedingedge/networks_${ARCH}.tar.gz"

it allows not to waste bucket storage when script modification triggers the bleedingedge upload even if there were no functional changes
it allows for testing the new changes in Autoware before tagging, potentially through the Autoware unit tests of the (future) packages using neural networks

On pushes of tags (X.Y.Z):

continue to push docker image as “latest”

it should be smart enough and reuse the same image if there were no changes

upload artifacts to "/X.Y.Z/networks_${ARCH}.tar.gz"

only on Z=0 according to the tagging rules

ACTION: modify CI files to reflect previous points
ACTION: create a document that explains the versioning logic and that can act as a guide for the maintainers
ACTION: update the header template to add the version field described in the  "Model descriptor" paragraph

Autoware.Auto

Change "/networks_${ARCH}.tar.gz" to "/X.Y.Z/networks_${ARCH}.tar.gz" in the neural_networks package to align with the change in the bucket

where “X.Y.Z” would be stored in a dedicated file for easy maintenance

ACTION: make that change
ACTION: document the way the version field in the descriptor can be used and/or make use of it in the tvm_utility test cases as  an example

Notes

S3 buckets have an integrated versioning system, but it is not practical for our use case.
This change will be redundant with the current use we have of a dedicated hash file for the archive in the bucket, unless we want to keep it for integrity checks after the download step (but that's not currently done).

How modelzoo works right now
After the models are compiled,

here the models get compressed all together:

https://github.com/autowarefoundation/modelzoo/blob/master/.github/workflows/compilation-push-master.yaml#L43-L45

and here the models get uploaded:

https://github.com/autowarefoundation/modelzoo/blob/master/.github/workflows/compilation-push-master.yaml#L53-L57

in the Autoware.Auto (soon to be in Autoware Universe) it gets downloaded and extracted with externalproject_add:

https://gitlab.com/autowarefoundation/autoware.auto/AutowareAuto/-/blob/master/src/common/neural_networks/CMakeLists.txt#L37

Right now we have about 5 models and they occupy 558MB space when compressed for each architecture in the S3 bucket.
Problems

As the number of these models grow, this single file will get way too big. It's not scalable.
There is no way to only download the model user needs.

Solution
We should utilize folders in S3 for this use case. We can copy the structure of the tensorflow model zoo for this.
S3 cp command instructions explain how we can push folders recursively.
Here is an example file path there:

http://download.tensorflow.org/models/object_detection/tf2/20200713/centernet_hg104_512x512_coco17_tpu-8.tar.gz

We could have something like this:

bucket_dir/models/model_name/date_as_version/model_arch/model_name-date_as_version-model_arch

Extra:

We should make it clear what files are available by automatically generating a .md file table that lists the models generated with their versions, archs, names.
In autoware, we will link directly to these model files, if new version of a model comes out, it must be tested and updated manually on autoware side.

@ambroise-arm @luca-fancellu @esteve what do you think?

bucket_dir/models/model_name/date_as_version/model_arch/model_name-date_as_version-model_arch

I think it would be more convenient to use the version of the ModelZoo repository instead of the date. That way it would reflect if a major change happened on the MZ side, and help align API-breaking changes. And also to easily checkout MZ to the associated tag, if that's needed for any reason.

@ambroise-arm will the contents of a given version of ModelZoo change? If so, I'd prefer using date_as_version as @xmfcx proposed. That way it's easier to debug users' issues and track what everyone is using. If, on the other hand, the contents of a given version are frozen, then it's fine to use either, in my opinion.

@esteve No, it won't change. A ModelZoo version is a tagged commit of this repository, with a fixed version of the source models (as they are held as lfs files) and fixed artifacts (as they are compiled once by the CI on tagging and then uploaded to the bucket). And even the compilation environment is fixed (as the CI pushes a tagged docker image here) if someone wants to recompile older models.

@ambroise-arm in that case, perhaps we could add a suffix with the date, just for informative reasons, e.g. 1.2.3-20220504


Fix FFI type signatures for write/end functions

Description of the change
Properly fixes #35

Checklist:

[ ] Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
[ ] Linked any existing issues or proposals that this pull request should close
[ ] Updated or added relevant documentation
[ ] Added a test for the contribution (if applicable)

@garyb When this does build, mind giving this an approval?
The tests pass locally, so I'm not sure why they don't pass on CI.


commits_count key for changelog_from_git_commits action

#4555 Hi! There is very useful changelog_from_git_commits action. But sometimes I need just a changelog from last N commits(for example in some of my cases I don't know what commits should I use for :between key). So, I've added new key to changelog_from_git_commits called :commits_count.

@SiarheiFedartsou Thanks for working with us. I see we've already engaged you on the PR. I hope you continue to work with us there. Thanks!


CSharp parameterless constructor for non-zero Structs like GDScript

Change CSharp to use parameterless constructors to match GDScript.
If the old behavior is prefered please close this pull request.
If this should be merged the docs C# API differences to GDScript must be updated.

Did you test it? As far as I know, this is impossible.

However, I'm not sure if this is something we want, I guess it can be confusing since this was never possible until very recently but I have no strong opinion either way. I don't know if this might be unintuitive or break developers' expectations.

My focus was the same behavior between GDScript and CSharp.
I don't know why the zero-values of these Types should be used, they all make only sense to me with non-zero values.

The two concerns I have are:

I'm not entirely convinced this change in C# was a good thing. I always wanted custom parameter-less constructors for structs, but the fact that now default(Basis) and new Basis() result in different values can be confusing and cause mistakes that are hard to detect.
This changes behaviour from 3.x, which can be very confusing. It will make upgrading code bases harder as it will need to be done more carefully.

I'm more inclined to make the constructor private (is that possible now?) and introduce this change in Godot 5, which should make it easier to upgrade as most people will do so from 4.x by then.
That being said, we should gather feedback from users to know what they think.

the fact that now default(Basis) and new Basis() result in different values can be confusing and cause mistakes that are hard to detect.

That's exactly what makes me hesitant about using parameter-less constructors in structs so I tend to avoid them in my personal projects, but I'm not against it if users want it.

I'm more inclined to make the constructor private (is that possible now?)

It's not possible, if a struct doesn't implement the parameter-less constructor it behaves as before but if you do implement it then it must be public.

Would be nice if everyone could vote with thumbs up and thumps down on this comment to see the average opinion.

I'm not entirely convinced this change in C# was a good thing. I always wanted custom parameter-less constructors for structs, but the fact that now default(Basis) and new Basis() result in different values can be confusing and cause mistakes that are hard to detect.

I entirely second that.

Closed because the current behaviour is prefered.