[SPARK-19583][SQL]CTAS for data source table with a created location should succeed

What changes were proposed in this pull request?
  spark.sql(
          s"""
             |CREATE TABLE t
             |USING parquet
             |PARTITIONED BY(a, b)
             |LOCATION '$dir'
             |AS SELECT 3 as a, 4 as b, 1 as c, 2 as d
           """.stripMargin)

Failed with the error message:
path file:/private/var/folders/6r/15tqm8hn3ldb3rmbfqm1gf4c0000gn/T/spark-195cd513-428a-4df9-b196-87db0c73e772 already exists.;
org.apache.spark.sql.AnalysisException: path file:/private/var/folders/6r/15tqm8hn3ldb3rmbfqm1gf4c0000gn/T/spark-195cd513-428a-4df9-b196-87db0c73e772 already exists.;
	at org.apache.spark.sql.execution.datasources.InsertIntoHadoopFsRelationCommand.run(InsertIntoHadoopFsRelationCommand.scala:102)

while hive table is ok ,so we should fix it for datasource table.
The reason is that the SaveMode check is put in  InsertIntoHadoopFsRelationCommand , and the SaveMode check actually use path, this is fine when we use DataFrameWriter.save(), because this situation of SaveMode act on path.
While when we use  CreateDataSourceAsSelectCommand, the situation of SaveMode act on table, and
we have already do SaveMode check in CreateDataSourceAsSelectCommand for table , so we should not do SaveMode check in the following logic in InsertIntoHadoopFsRelationCommand for path, this is redundant and wrong logic for CreateDataSourceAsSelectCommand
How was this patch tested?
unit test added

Test build #72932 has started for PR 16938 at commit 058865b.

Test build #72932 has finished for PR 16938 at commit 058865b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72932/
Test PASSed.

cc @gatorsmile  @cloud-fan

I don't think we should treat it as a bug just because hive supports it, we should think more. Does it make sense to specify an existing directory in CTAS?

We need to define a consistent rule in Catalog how to handle the scenario when the to-be-created directory already exists. So far, in most DDL scenarios, when trying to create a directory but it already exists, we just simply use the existing directory without an error message. mkdir does not complain if the destination directory exists.

From what I understand, this change is applicable for EXTERNAL tables only.
There are two main uses of EXTERNAL tables I am aware of (repost from https://github.com/apache/spark/pull/16868#issuecomment-279282420):

Ingest data from non-hive locations into Hive tables.
Create a logical "pointer" to an existing hive table / partition (without creating multiple copies of the underlying data).

Ability to point to random location (which already has data) and create an EXTERNAL table over it is important for supporting EXTERNAL tables. If we don't allow this PR, then the options left to users are:

Create an external table and point to some non-existing location.
Later do either of these 2 things:

issue ALTER TABLE SET LOCATION to set the external table's location to the source location having desired data.
do a dfs -mv from the source location of the data to the new location which the table points at. This will be nasty in case your source data was a managed table location.

@cloud-fan : I don't think Spark's interpretation of EXTERNAL tables is different from Hive's. If it is, can you share the differences ? I think we should allow this. If you have specific concerns, lets discuss those.

I think in CTAS，it is not allowed an existed table，  no strict for the path exists. In DataFrameWriter.save with errorifnotexist mode，path existed is not allowed.

@cloud-fan @gatorsmile @tejasapatil  let's discuss this together?

ok let's discuss it case by case:

CREATE TABLE ... LOCATION path works if path exists, it's expected
CREATE TABLE ... LOCATION path fails if path doesn't exist, is it expected?
CREATE TABLE ... LOCATION path AS SELECT ..., shall we fail if path exists?
ALTER TABLE ... SET LOCATION path, shall we fail if path not exist?

One more case:
5. CREATE TABLE or CTAS without the location spec: if the default path exists, should we succeed or fail?
After we finishing the TABLE-level DDLs, we also need to do the same things for DATABASE-level DDLs and PARTITION-level DDLs.

1. CREATE TABLE ... LOCATION path
a) path exists
      hive -> ok
      spark -> ok
b) path not exists
      hive -> ok
      spark -> throw exception(path does not exists)

2. CREATE TABLE ... LOCATION path AS SELECT ...
a) path exists
	  hive -> throw exception(CREATE-TABLE-AS-SELECT cannot create table with location to a non-empty directory.)
	  spark -> throw exception(path already exists)
b) path not exists
      hive -> ok
      spark -> ok

3. ALTER TABLE ... SET LOCATION path
a) path exists
	   hive -> ok
	   spark -> ok
b) path not exists
	   hive -> ok
	   spark -> ok

**4. CREATE TABLE ... **
a) default warehouse table path exists
       hive -> ok
       spark -> ok
b) default warehouse table path not exists
       hive -> ok
       spark -> ok

5. CREATE TABLE ... AS SELECT ...
a) default warehouse table path exists
       hive -> ok
       spark -> throw exception(path already exists)
b) default warehouse table path not exists
       hive -> ok
       spark -> ok

Compare spark-master branch and hive-2.0.0
1. CREATE TABLE ... PARTITIONED BY ... LOCATION path
a) path exists
      hive -> ok
      spark -> ok
b) path not exists
      hive -> ok
      spark -> throw exception(path does not exists)

2. CREATE TABLE ...PARTITIONED BY ... LOCATION path AS SELECT ...
a) path exists
	  hive -> not support
	  spark -> throw exception(path already exists)
b) path not exists
      hive -> not support
      spark -> ok

3. ALTER TABLE ... PARTITION(...) ... SET LOCATION path
a) path exists
	   hive -> ok
	   spark -> ok
b) path not exists
	   hive -> ok
	   spark -> ok

4. CREATE TABLE ....PARTITIONED BY ...
a) default warehouse table path exists
       hive -> ok
       spark -> ok
b) default warehouse table path not exists
       hive -> ok
       spark -> ok

5. CREATE TABLE ... AS SELECT ...
a) default warehouse table path exists
       hive -> not support
       spark -> throw exception(path already exists)
b) default warehouse table path not exists
       hive -> not support
       spark -> ok

@windpiger Thank you for your efforts! What you did above need to be written as the test cases. Could you do it as a separate PR?
In addition, all the cases you tried are only for hive serve tables, right?

Could you check the behaviors for both data source tables and hive serde tables? Later, we also need to check the behaviors of InMemoryCatalog for data source tables without enabling Hive supports.

@gatorsmile
Sorry, I forget to declare that ,Above tests, spark represents parquet table with HiveExternalCatalog , hive represents hive table in hive2.0.0.
I will add hive serde table for spark with HiveExternalCatalog， and parquet/hive serde table with InMemoryCatalog soon.

@gatorsmile  I have test all the cases above updated. The result shows that
spark for datasource table with HiveExternalCatalog and InMemoryCatatlog have the same actions.
spark for hive table passed all the tests above.
there are some different between spark for hive table and spark for datasource table.

@windpiger :

what does throw exception(...) mean ? Operation is supported OR not ? it might throw exception but the operation itself might have happened.
for 2nd point, you said hive does not support that. Can you share the error message ? I am trying to understand if there is a reason why hive does not allow that and with Spark we would also need to think about that.
I could not understand what default warehouse table path exists in 3rd point means

CREATE TABLE ... (PARTITIONED BY ...) LOCATION path

I think hive's behavior makes more sense. Users may wanna insert data to this table and put the data in a specified location, even it doesn't exist at the beginning.

CREATE TABLE ...(PARTITIONED BY ...) LOCATION path AS SELECT ...

The reason applies here too.

CREATE TABLE ... (PARTITIONED BY ...) AS SELECT ...

When users don't specify the location, mostly they would expect this is a fresh table and the table path should not exist.

@tejasapatil

throw exception is the result of the test, It is really happened in current spark master branch

Hive CTAS not support for partition table hive-doc

default warehouse table path exist means that we already create a table path under warehouse path, before we create the table.

@cloud-fan
situation 2. CREATE TABLE ...(PARTITIONED BY ...) LOCATION path AS SELECT ...
is different for path exists, which is this PR going to resolve. It is ok to make it consist with hive with HiveExternalCatalog in spark?
situation 3. CREATE TABLE ... (PARTITIONED BY ...) AS SELECT ...
is also different for default warehouse table path exists, do you mean that the parquet action is expected that throw an already exist exception, and hive should make consist with it?

@windpiger yes for both questions.

Based on the doc, Hive does not support CTAS when the target table is external.
1. CREATE TABLE ... LOCATION path
2. CREATE TABLE ... LOCATION path AS SELECT ...
When you testing the above two cases, you need to change the syntax a little bit by manually adding EXTERNAL. Spark SQL actually is creating an external table. This is different from Hive.
1. CREATE EXTERNAL TABLE ... LOCATION path
2. CREATE EXTERNAL TABLE ... LOCATION path AS SELECT ...
Could you update the comparison? Thanks!

@gatorsmile sorry, I make a mistake of this, I have updated the compare test above.

@cloud-fan @gatorsmile @tejasapatil  As we discussed above, we have three actions to do:
1. CREATE TABLE ... (PARTITIONED BY ...) LOCATION path
situation:path not exists

Item
Before
After

spark(parquet with HiveExternalCatalog)
throw exception(path does not exists)
ok

spark(parquet with InMemoryCatalog)
throw exception(path does not exists)
ok

2. CREATE TABLE ...(PARTITIONED BY ...) LOCATION path AS SELECT
situation:path exists

Item
Before
After

spark(parquet with HiveExternalCatalog)
throw exception(path already exists)
ok

spark(parquet with InMemoryCatalog)
throw exception(path already exists)
ok

3. CREATE TABLE ... (PARTITIONED BY ...) AS SELECT ...
situation:default warehouse table path exists

Item
Before
After

spark(hive with HiveExternalCatalog)
ok
throw exception(path already exists)

please help to confirm the actions above, if it is ok, situation 2 is this PR going to resolve, and I will make another PR to resolve situation 1&3, thanks~

I found you also changed the following cases:
4. CREATE TABLE ....
5. CREATE TABLE ... AS SELECT ...
Actually, they are managed tables. You do not need to update them. Can you roll back the changes? Thanks!

Basically, the rules you proposed are

When users specify the location in CT or CTAS (i.e., creating an external table), we should create a new directory if not existed, or overwrite the directory if already created.
When users do not specify the location in CT or CTAS (i.e., creating a manged table), we should create a new directory if not existed. If the directory already exists, we should issue the error.

Is my understanding right?

oh, you are right~ thanks!

Thank you for your work!
Maybe the last question.
**2. CREATE TABLE ...PARTITIONED BY ... LOCATION path AS SELECT ...**
a) path exists
	  hive(external) -> not support
          spark(hive with HiveExternalCatalog) -> ok
	  spark(parquet with HiveExternalCatalog) -> throw exception(path already exists)
          spark(parquet with InMemoryCatalog) -> throw exception(path already exists)

In the above case, you used path exists. I assumed this is the existence of the table directory. Are these behaviors still the same when the specific partition directory exists?

@windpiger : I realised that you are checking the hive behavior against Hive 2.0.0. Spark is expected to support semantics for Hive 1.2.1 : https://github.com/apache/spark/blob/3881f342b49efdb1e0d5ee27f616451ea1928c5d/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala#L58
I am not upto date with the differences between those two releases of hive wrt this discussion. Can you confirm if the observations reported earlier in the discussion are valid against Hive 1.2.1 ?

@tejasapatil In my opinion, test in Hive 2.0.0 just make a compare with Spark, the target is to determine these actions in Spark, not to make consist with Hive 2.0.0 or Hive 1.2.1, isn't it?

I looked into the code. Looks like that version is merely for picking the hive shim and metastore interactions and got nothing to do with semantics of SQL operations. So you are most likely correct.
@gatorsmile @cloud-fan : Is the goal of hive support in spark to be adherent with a specific release of Hive (as long as the hive behavior is sane and consistent .. otherwise it doesn't make sense to follow it) ?

@tejasapatil Spark doesn't need to be exactly same with Hive, we follow hive behavior if it's reasonable, or use our own logic if hive's behavior doesn't make sense.

@gatorsmile I have test it for partition path exists , the result is still same with table path exists
2. CREATE TABLE ...PARTITIONED BY ... LOCATION path AS SELECT ...
a) path exists
hive(external) -> not support
spark(hive with HiveExternalCatalog) -> ok
spark(parquet with HiveExternalCatalog) -> throw exception(path already exists)
spark(parquet with InMemoryCatalog) -> throw exception(path already exists)

Test build #73399 has started for PR 16938 at commit 8559e4e.

Test build #73400 has started for PR 16938 at commit afa1313.

Test build #73405 has started for PR 16938 at commit 1f2ce17.

Test build #73400 has finished for PR 16938 at commit afa1313.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73400/
Test FAILed.

Test build #73405 has finished for PR 16938 at commit 1f2ce17.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73405/
Test FAILed.

Test build #73411 has started for PR 16938 at commit 5a3e5ac.

Build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73399/
Test FAILed.

Test build #73411 has finished for PR 16938 at commit 5a3e5ac.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73411/
Test PASSed.

Test build #73424 has started for PR 16938 at commit 416ea37.

Test build #73424 has finished for PR 16938 at commit 416ea37.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73424/
Test PASSed.

@gatorsmile @cloud-fan could you help to review this pr? thanks :)

Test build #73587 has started for PR 16938 at commit 304ae31.

Test build #73587 has finished for PR 16938 at commit 304ae31.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73587/
Test PASSed.

Test build #73634 has started for PR 16938 at commit 2498dfd.

Test build #73634 has finished for PR 16938 at commit 2498dfd.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73634/
Test FAILed.

Merged build finished. Test FAILed.

Test build #73648 has started for PR 16938 at commit a8dbcca.

Test build #73648 has finished for PR 16938 at commit a8dbcca.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73648/
Test PASSed.

Merged build finished. Test PASSed.

I am modifying the hacky code

Test build #73680 has started for PR 16938 at commit d78b7d5.

Test build #73680 has finished for PR 16938 at commit d78b7d5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73680/
Test PASSed.

Merged build finished. Test PASSed.

thanks, merging to master!


Enable ActiveSupport::NumericWithFormat mix-in

Fix  #96
AS::NumericWithFormat#to_s still accepts untyped as arguments, but I think it is acceptable for now. If someone want to make the type stricter, open a pull request.

Well, maybe this patch doens't work expectedly becasue the included AS::NumericWithFormat#to_s is ignored by the original Integer#to_s. We need prepend but RBS doesn't have.


Implement hb_mc_device_sync

We need a function for waiting on kernel completion!
(Thanks to @bornaehsani for catching this!)

Out of date. Updated version addressed in PR #251 .


Incorrect positioning and appearance of comment buttons with TimeField and DateTimeFields

Issue Summary
There are two, possibly related issues here.
TimeField and DateTimeFields show comment buttons below the field rather than beside the field:

TimeField and DateTimeFields in an Orderable (inline panel) show comment buttons when they shouldn't (see #9649 and #9641):

Note, however, that these comment buttons only occur on an unsaved inline panel; one the page is saved, inline panels rendered initially are correct (ie. no comment buttons), whereas newly created inline panels incorrectly have the comment buttons:

Steps to Reproduce
Common Steps to Reproduce

Start a fresh Wagtail installation as per the instructions at Getting Started.
Merge in the application at https://github.com/dkirkham/wagtail-ui-testpage
Add 'testpage' to INSTALLED_APPS in settings/base.py
Migrate and runserver
Login to the Wagtail admin interface
Further steps as described below

For the first example:

Create a child Testpage of the home page
Go to the Title & Introduction tab (the default) and hover the mouse/pointer underneath both the Time and DateTime fields
Observe that the comment buttons appear under the field; they should be horizontally adjacent to the field (ie right hand side, in LTR languages)
Observe that the comment buttons work correctly, creating a comment box on the right.

For the second example:

Create a child Testpage of the home page
Go to the Event Details tab and click the + Add orderables button
Observe that the comment buttons appear under the field; they shouldn't be present given #9649
Observe that the comment buttons are sort of correctly ineffective; nothing happens when they are clicked
Enter data for the time and datetime, and for other fields (note the test project has some mandatory fields and a dummy Test sub page will also be required before saving as a draft is possible)
Save the page
Observe the saved inline panels on the Event Details tab; the comment buttons do not appear
Click the + Add orderables button to create another inline panel
Observe that the comment buttons appear under the field; again, they shouldn't be present given #9649

Any other relevant information. For example, why do you consider this a bug and what did you expect to happen instead?

In the first example, this appears not to conform with the intended positioning of comment buttons, usually adjacent to the relevant field.
In the second example, the comment buttons should not appear at all, at least until commenting with Orderables/inline panels is addressed generally

I have confirmed that this issue can be reproduced as described on a fresh Wagtail project: yes
Technical details

Python version: 3.11
Django version: 5.0
Wagtail version: 6.2
Browser version: Firefox 128, Chrome 127, Safari 17.4.1 on Mac Sonoma

Although not tested with the testpage app, I have observed the same issues in a production deployment using Wagtail 5.1.3, Django 4.2.11 and Python 3.8.
Working on this

Anyone can contribute to this. View our contributing guidelines, add a comment to the issue once you’re ready to start.

Hi @dkirkham @laymonage @thibaudcolas ,
I have reproduced the issue:
For the first example: On the child Testpage of the home page, I went to the Title & Introduction tab (the default) and observed that the comment button is there but below the inline panel not adjacent to it :

For the second example: On the child Testpage of the home page, I went to the Event Details tab, clicked the + Add orderables button, and observed that the comment button is there but it is ineffective:

After saving, I couldn't see this button anymore:

again If I create another inline panel by clicking the + Add orderables button, the ineffective comment button appears again and disappears after saving:

Technical Details:
Python version: 3.13.0
Django version: 5.1.4
Wagtail version: 6.3.1
Browser version:  Chrome 131 on Windows 11
I would like to work on this issue.
Please confirm , after making the changes:

For the first example, the comment buttons should be adjacent to the relevant field on the Title & Introduction tab.
For the second example, when clicking the + Add orderables button on the Event Details tab, comment buttons should not appear at all.

Is there anything else I am missing?

I believe this is correct, at least until there is a separate effort to add the ability to comment against Orderable fields.

I can confirm that this incorrect positioning appears not only in TimeField and DateTimeField but also in DateField, as I can see on the Blog page in both the bakerydemo and this  installation.


maeparser 1.2.4

Reference Issue

What does this implement/fix? Explain your changes.
Any other comments?

Thanks @sailfish009!


create a group buy list

Hello,
so i'd love to support the project by buying directly from the maker but shipping to the european continent plus import taxes makes it incredibly expensive.
Maybe instead we could set up a list where interested people in different parts of the world can act as points of contact and buy for all the locally interested people

Thanks for your interest. Just for the record, I don't sell these myself (but I am based in Germany).
The best bet is to get 5pcs made by JLCPCB, I think it is around 120€ or so, and you have 4 others to sell/give away.
I think this platform is not ideal for a group buy list, since people usually only come here when they have issues. I suggest you can visit the Discord channel (linked on the GitHub page). Unfortunately, it is mostly populated with US people.

Which country do you live in?

Due to inactivity I am going to close this issue. Feel free to reopen if required.


mindustry-server: Fix

Description
Fix mindustry-server build after https://github.com/NixOS/nixpkgs/pull/221396
Things done

Built on platform(s)

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

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

[ ] Tested, as applicable:

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

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
Can't:
earlyoom: mem avail:   155 of  7829 MiB ( 1.99%), swap free: 1047 of 10540 MiB ( 9.94%)
earlyoom: low memory! at or below SIGTERM limits: mem 10.00%, swap 10.00%
earlyoom: sending SIGTERM to process 490532 uid 1000 "nix-env": badness 1093, VmRSS 5789 MiB

[ ] Tested basic functionality of all binary files (usually in ./result/bin/)

23.05 Release Notes (or backporting 22.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[x] Fits CONTRIBUTING.md.

@GrahamcOfBorg build mindustry mindustry.tests.nixosTest


Problems when using custom learning rate scheduler for NeuralForecast Models

What happened + What you expected to happen
Hi together,
first, thank you so much for the implementation of a custom lr scheduler!
I ran into problems employing a ReduceLROnPlateau  scheduler.
Currently I am passing through the lr_scheduler and  lr_scheduler_kwargs parameters. However, I get the below error when implementing ReduceLROnPlateau. Other schedulers, like OneCycle, work perfectly though!  I think some of your earlier discussion evolved around it, however, I couldn't make it work yet.
[(https://github.com/Nixtla/neuralforecast/pull/998)]
MisconfigurationException('The lr scheduler dict must include a monitor when a ReduceLROnPlateau scheduler is used. For example: {"optimizer": optimizer, "lr_scheduler": {"scheduler": scheduler, "monitor": "your_loss"}}').
Versions / Dependencies
GoogleColab
neuralforecast
Version: 1.7.4
Reproduction script
See above - if needed happy to supply more details.
Issue Severity
Medium: It is a significant difficulty but I can work around it.

Maybe @JQGoh, @jmorales, @BrunoBelucci you have some idea of how to solve it?

Hello! Can you send a code snippet of OneCycle that works and one for ReduceLROnPlateau to reproduce on my end? Thanks!

Hello,
To the best of my knowledge, it is not currently possible to use ReduceLROnPlateau out-of-the-box with the existing implementation of neuralforecast. As mentioned in this comment, this would require the user to provide a "lr_scheduler_config," which is not directly supported at this time. This "lr_scheduler_config" should be returned by the configure_optimizers method in Lightning (refer to this documentation for more details).
In the "lr_scheduler_config," you need to specify the "monitor" parameter with the metric you're tracking to detect the plateau. Typically, the "interval" is also set to "epoch" because the plateau is usually identified per epoch rather than per step, though this may vary depending on your model.
Currently, I am not using neuralforecast with ReduceLROnPlateau, so I cannot offer an immediate solution. However, based on our previous discussion in #998, I believe the simplest workaround for now is to monkey-patch the configure_optimizers function of your model to return a dictionary as expected by Lightning. This dictionary should include an "optimizer" key and, optionally, a "lr_scheduler" key, which can be a single LR scheduler or a lr_scheduler_config (as specified in the documentation).
If you do not manage to solve this, you can provide me a minimum (not) working example and I can see if I can fix it for you during my free time.
Regards,
Bruno

Thank you so much for your explanations and looking into how to resolve the issue!!
Below a code snippet that (a) works with OneCycleLR and (b) that reproduces the error with ReduceLRonPlateau.
# GENERAL + DATA-PROCESSING
!pip install neuralforecast
from neuralforecast import NeuralForecast
from neuralforecast.models import NHITS
from neuralforecast.utils import AirPassengersDF

Y_df = AirPassengersDF # Defined in neuralforecast.utils
Y_df.head()

a.  - OneCycleLR
from torch.optim.lr_scheduler import OneCycleLR

lr_scheduler_cls = OneCycleLR
max_lr = 1e-2
lr_scheduler_kwargs = {
    'max_lr': max_lr,
    'total_steps': 50}

horizon = 12

models = [ NHITS(h=horizon,
                input_size=2 * horizon,
                max_steps=50,
                lr_scheduler = lr_scheduler_cls,
                lr_scheduler_kwargs = lr_scheduler_kwargs) ]
nf = NeuralForecast(models=models, freq='M')
nf.fit(df=Y_df)

b. -ReduceLROnPlateau

from torch.optim.lr_scheduler import ReduceLROnPlateau
lr_scheduler_cls = ReduceLROnPlateau
lr_scheduler_kwargs = {
      'mode': 'min',
      'factor': 0.5,
      'patience': 2}

horizon = 12

models = [ NHITS(h=horizon,
                input_size=2 * horizon,
                max_steps=50,
                lr_scheduler = lr_scheduler_cls,
                lr_scheduler_kwargs = lr_scheduler_kwargs) ]
nf = NeuralForecast(models=models, freq='M')
nf.fit(df=Y_df)

ERROR MESSAGE

/usr/local/lib/python3.10/dist-packages/pytorch_lightning/core/optimizer.py in _configure_schedulers_automatic_opt(schedulers, monitor)
275             )
276             if scheduler["reduce_on_plateau"] and scheduler.get("monitor", None) is None:
--> 277                 raise MisconfigurationException(
278                     "The lr scheduler dict must include a monitor when a ReduceLROnPlateau scheduler is used."
279                     ' For example: {"optimizer": optimizer, "lr_scheduler":'
MisconfigurationException: The lr scheduler dict must include a monitor when a ReduceLROnPlateau scheduler is used. For example: {"optimizer": optimizer, "lr_scheduler": {"scheduler": scheduler, "monitor": "your_loss"}}

Thank you very much @JQGoh!! I will use this dev-version and your example for implementation 👍 This flexibility in configuring the optimizers is amazing :)

@JQGoh I tried to manipulate the set_configure_optimizers  as you described. For that reason I (tried to) install the developer version of nixtla as per below. Maybe I do this wrongly. I cannot see the set_configure_optimizers  parameter in the imported neuralforecast models nor use it to implement LR on plateau. Do you have an idea where the problem resides?
# install DEV-version of nixtla (yet not sucessfull?)
!git clone https://github.com/Nixtla/neuralforecast.git
%cd neuralforecast
!pip install -e

#
from torch.optim.lr_scheduler import ReduceLROnPlateau
optimizer = torch.optim.Adam(LSTMm.parameters(), lr=learning_rate)
scheduler = torch.optim.lr_scheduler.ReduceLROnPlateau(optimizer = optimizer, mode='min', factor=0.5, patience=2)

model.set_configure_optimizers(optimizer=optimizer,scheduler=scheduler,monitor="train_loss")

Hey. I think we can introduce that method and deprecate the arguments

@jmoralez That sounds great😊


Data gets overwritten in Backend when not refreshed

I just experienced a surprising behaviour.
I made an update in data through the API. But I had the database viewer open in safari. I clicked on the updated record (without refreshing first) - the record kept the outdated data in the detail view. hitting safe overwrites the updated data.
nothing very serious but wasn't expecting that the record data is not refreshed when I open a record in detail view.

The Admin UI doesn't listen for realtime events at the moment.
This may change in the future with the planned Admin UI rewrite but I already have enough on my hands and for now this remains out of the scope.

I dont mean that a realtime update is needed - I was just wondering that opening the details page of a record is not reloading the data. but I know that this is probably never happening in production anyway.
AND: this is an amazing tool already!! I enjoy it so much. so thanks

I was just wondering that opening the details page of a record is not reloading the data.

This is already how it works in v0.23.0-rc (you can inspect the requests in the live demo https://pocketbase.io/demo/) but again there are no realtime listeners at the moment and it doesn't prevent submitting updates with stale data (after I manage to complete most of the other v0.23.0 remaining tasks I'll consider at least adding a warning before save if we detect that a new version exist).


Timestamp field, Timezone conversion?

Hello, and thank you for your amazing work on this project!
 My app uses UTC date/time timestamps for created_at / updated_at and other timestamp fields, however I'd like to convert those to a local timezone within motor-admin. Ideally I'd like to use the user's browser timezone, but even hard-coding a single timezone in the rails config or motor.yml would be adequate for now (config.time_zone does not seem to be honored). The biggest requirement is that I'd like to keep the UTC timestamps in the database / activerecord models, but display them in something a bit more readily human readable in motor-admin.
Currently I'm using unique select statements for each resource, where I'm doing the conversion in the DB with CONVERT_TZ(), then hiding/renaming columns for the user... which I'm not super proud of.
Is there anything simpler that you can suggest I try?

thanks @Igneous, i totally agree - it makes sense to use user's browser timezone to display time.
It has been fixed in the newest release: https://rubygems.org/gems/motor-admin/versions/0.1.100
Please let me know if this solves the issue for your application.

Wow, honestly I was not expecting a response so quickly, let alone a fix. It works brilliantly, thanks @omohokcoj!


generic CMake on Linux

I'm hitting multiple issues using generic CMake (no package managers) on Fedora 36 and Ubuntu 22.  I can make it work with changes.
Are you open to CMake changes/PRs?

Sure, I am also happy to look into it myself if you want. I personally use Ubuntu 20.04 + vcpkg with GCC and Clang just fine.
I am particularly interested in having asio-grpc CMake install work properly on all systems (ASIO_GRPC_INSTALL) along with the CMake options (e.g. ASIO_GRPC_USE_BOOST_CONTAINER). When it comes to compiling the tests and examples then I do not expect things to work without vcpkg or pkg-config, but if they can be made to work without significant changes then I am happy to integrate them.

It's fairly straight forward to replicate.  Either Ubutnu 22 or Fedora 36, with stock version of CMake.
I use CLion for projects, which behaves the same as CMake at command line.
In all my use cases vcpkg or other would never be used, only generic CMake.

Ok, I was able to spot that asio-grpc gets mis-installed by some CMake generators (apparently Unix Makefiles), which cause memory_resource.hpp to be incorrect. I fixed that on master now.  -DCMAKE_INSTALL_PREFIX=/out &&\
    cmake --build build --target install

COPY ./src /src

WORKDIR /src

RUN cmake -Bbuild . -DCMAKE_PREFIX_PATH=/out -DCMAKE_POSITION_INDEPENDENT_CODE=on &&\
    cmake --build build

ENTRYPOINT ["/src/build/test"]

and src/CMakeLists.txt:
cmake_minimum_required(VERSION 3.15)

project(test)

find_package(PkgConfig)

pkg_check_modules(protobuf REQUIRED IMPORTED_TARGET protobuf)

add_executable(protobuf::protoc IMPORTED)
find_program(PROTOC_EXECUTABLE protoc)
set_target_properties(protobuf::protoc PROPERTIES IMPORTED_LOCATION
                                                  "${PROTOC_EXECUTABLE}")

add_executable(gRPC::grpc_cpp_plugin IMPORTED)
find_program(GRPC_CPP_PLUGIN_EXECUTABLE grpc_cpp_plugin)
set_target_properties(
  gRPC::grpc_cpp_plugin PROPERTIES IMPORTED_LOCATION
                                   "${GRPC_CPP_PLUGIN_EXECUTABLE}")

pkg_check_modules(grpc++ REQUIRED IMPORTED_TARGET grpc++)
pkg_check_modules(grpc REQUIRED IMPORTED_TARGET grpc)

find_package(asio-grpc CONFIG QUIET)

add_executable(test main.cpp)

target_link_libraries(test PRIVATE asio-grpc::asio-grpc PkgConfig::protobuf
                                   PkgConfig::grpc++ PkgConfig::grpc)

asio_grpc_protobuf_generate(
  GENERATE_GRPC
  TARGET
  test
  IMPORT_DIRS
  "${CMAKE_CURRENT_SOURCE_DIR}"
  PROTOS
  "${CMAKE_CURRENT_SOURCE_DIR}/helloworld/helloworld.proto")

target_compile_features(test PRIVATE cxx_std_20)

with src/main.cpp and src/helloworld/helloworld.proto works fine. Using pkg-config makes things a bit clumpsy and I highly recommend using a more recent version of gRPC, the one provided by Ubuntu is only 1.30.2 (from Jul 18, 2020).

I'll take a look at this.
I tend to prefer CMAKE_STAGING_PREFIX=pwd/out/usr over setting `CMAKE_INSTALL_PREFIX, as it correctly manages rpath, and in above case the install is technically a staged install.
Is 20 a hard requirement?  It would be useful to see some analysis between using 17 vs 20.  Picking up 20 just for the co-routine convenience is a hard sell for many.
I appreciate the assistance.

Thank you for looking into it.
I didn't know about CMAKE_STAGING_PREFIX, seems interesting.
No, C++17 is fully supported. Looks like I have just have to be very careful with any example/tutorial code that I share anywhere 😁 to not give false impressions.

Documenting the recommended pattern to use your library with C++17 would be helpful.  Maybe even a dedicated example with a subfolder named as such.
Another is clang-tidy compliance.  CLion has this enabled by default.
Also a platform agnostic way to set some common flags I tend to always use:
set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_EXTENSIONS OFF) set(CMAKE_POSITION_INDEPENDENT_CODE ON)
I'm evaluating high perf gRPC/Flutter solutions for Automotive Grade Linux.

I'm also interested in using your library to replace the manual code in this library:
https://github.com/jwinarske/aasdk

Asio-grpc provides the glue-code between the async gRPC API (grpc::CompletionQueue) and a backend for expressing and composing asynchronous operations. Three such backends are available: Boost.Asio, standalone Asio and libunifex. It does not dictate how those backends are to be used and it is often best to see their respective documentations for more information.
Assuming that you are mostly interested in client-side code, you can find an example that uses Boost.Asio+Boost.Coroutine for a generic unary RPC here: https://github.com/Tradias/asio-grpc/blob/master/example/generic-client.cpp#L49-L53. In fact, you can take any C++20 coroutine example and replace co_spawn with spawn and use_awaitable with yield_context and it becomes C++17 compatible. For instance the helloworld client:
    asio::spawn(
        grpc_context,
        [&](const asio::yield_context& yield)
        {
            // ...
            status = RPC::request(grpc_context, stub, client_context, request, response, yield);
        },
        asio::detached);

For best possible performance you would probably be looking at using libunifex though, e.g. this server-streaming example. It can seem a bit overwhelming at first. I recommend taking a look at the standardization proposal and related talks from cppcon and other conferences to learn more. Note that libunifex requires C++20 when using MSVC but C++17 otherwise.
I am excited to see your interest coming from an Automotive Grade Linux background.

@Tradias Great info.  I studied up on libunifex, and it looks like a good fit.  I will have validated the CMake changes in the next couple of days.  Many thanks!

@jwinarske have you made progress on your validation? Any additional issues or pain points that I can help with?

Libunifex doesn't seem to have any recent commit. @Tradias is asio-grpc with libunifex stable to use in production?

@Tradias Not yet.  It's still on my radar

Libunifex is part of facebookexperimental:

This project is still evolving and should be considered experimental in nature. No guarantee is made for API or ABI stability.

However according to https://github.com/facebookexperimental/libunifex/issues/214

Most of Facebook's mobile apps on the major platforms now use libunifex.

And additionally:

It will probably never guarantee ABI stability. As parts of libunifex get standardized, I expect we'll declare those parts API-stable. That's probably all we're likely to guarantee, though.

Unfortunately libunifex requires a kernel version (5.6 or later) and it will not be available until RHEL 9.2. Can you please make a release available that can compile examples (tests not needed - I was able to compile all the tests) with C++ 20 or 17?

@smahapatra1 I am not sure I understand.
The current master branch will simply not create CMake targets that need libunifex when it cannot be found.
Any previous releases can be configured with ASIO_GRPC_ENABLE_CPP20_TESTS_AND_EXAMPLES=off to prevent the searching for- and creation of libunifex related targets.
I am surprised to hear that you managed to compile tests but not examples because both utilize libunifex in similar fashion.

@smahapatra1 I am not sure I understand.
The current master branch will simply not create CMake targets that need libunifex when it cannot be found. Any previous releases can be configured with ASIO_GRPC_ENABLE_CPP20_TESTS_AND_EXAMPLES=off to prevent the searching for- and creation of libunifex related targets.
I am surprised to hear that you managed to compile tests but not examples because both utilize libunifex in similar fashion.

Sorry, I was not clear. I am using tradias-asio-grpc-2.4.0 with -DASIO_GRPC_INSTALL=ON  -DASIO_GRPC_DISABLE_AUTOLINK=OFF -DASIO_GRPC_USE_BOOST_CONTAINER=ON -DASIO_GRPC_BUILD_TESTS=ON -DASIO_GRPC_ENABLE_CPP20_TESTS_AND_EXAMPLES=ON.
This fails with message:
-- Checking for module 'unifex'
--   Package 'unifex', required by 'virtual:world', not found
-- Configuring incomplete, errors occurred!
If I remove CPP20_TESTS_AND_EXAMPLES=ON but keep the rest (including ASIO_GRPC_BUILD_TESTS=ON), I can build it.
My goal is to turn off testing (after first successful test) and build the library and examples so that I can learn from the examples. I can't use libunifex.

ok, that should work in the next release where libunifex is treated as an optional dependency during example/test build + you can enable examples only with ASIO_GRPC_BUILD_EXAMPLES=on.
Also note that ASIO_GRPC_DISABLE_AUTOLINK has no effect when configuring asio-grpc. It is meant to be used before find_package(asio-grpc)/add_subdirectory(asio-grpc).

@smahapatra1 asio-grpc v2.5.0 has been released with:

Add a new CMake option called ASIO_GRPC_BUILD_EXAMPLES which enables compilation of just examples without tests. Also, libunifex is now treated as an optional dependency.

See release notes: https://github.com/Tradias/asio-grpc/releases/tag/v2.5.0 but use v2.5.1 in production.

@Tradias Looking at this again.  For the foreseeable future I'm only interested in libunifex support.  Is there a feasible combo for your libunifex examples and C++ 17?

Hi, thanks for checking back in. I am excited to hear that you are interested in libunifex. There are currently two examples:
client-side server-streaming
server-side unary

I hope your affords with libunifex are going well. Since the initial question in this issue has been resolved, I will close it. Please do not hesitate to create a new one if you have any more questions.


[Bug]: obsidian-git keeps adding and removing its own data.json file.

 The content of the file is the same each time. I think this file should either exist, or else it should not exist. I don't see why obsidian-git keeps deleting and recreating it.
Relevant errors (if available) from notifications or console (CTRL+SHIFT+I)
No response
Steps to reproduce
Simply leave obsidian running with obsidian-git installed and configured for automated periodic backups.
Expected Behavior
This file is either left in place, or else deleted and not recreated.
Addition context
No response
Operating system
Linux
Installation Method
AppImage
Plugin version
2.20.3

Are you using any additional syncing service on that directory? Is every commit affected by this or just some commits without any obvious pattern? I don't know how that could happen.

There's no other syncing service on that directory. It's just some commits without any obvious pattern. :( I'll see if I can catch it happening and see if there's anything in the console, etc.


localStream.updateConfiguration() doesn't work anymore

I don't know if is chrome related or not, but when I invoke localStream.updateConfiguration({maxVideoBW: somenumber}),
The result seems ok:
localStream.updateConfiguration({maxVideoBW: 3000, minVideoBW: 400})
erizo.js:175 DEBUG:  Maxvideo Requested 3000 limit 3000
erizo.js:175 DEBUG:  Result 3000
erizo.js:175 DEBUG:  Updating without SDP renegotiation, newVideoBW: 3000 newAudioBW: 100
erizo.js:175 DEBUG:  Sending message Object {type: "updatestream", sdp: "v=0...}
but in chrome://webrtc-internals bweforvideo doesn't change and the bandwidth remains the same as when I published the stream.
few months ago I'm 100% sure it worked flawlessly.
Problem in Chrome: 52.0.2743.116
UPDATE:
doesn't work in the last release of Chrome:  53.0.2785.89
works in Chromium: 55.0.2852.0 and Canary: 55.0.2851.0

It works for me in Chrome 53.0.2785.89.
There's one caveat though. You cannot update configuration to a BW higher than the one you set up initially with publish(stream, {maxVideoBW:}).
Give it a try and let me know if that is your case.

Pedro, now you mention it, while it is not possible to update BW to a
higher value, is it possible to update to a lower value and then update
again to the original one?
2016-09-06 6:50 GMT-03:00 Pedro Rodriguez<EMAIL_ADDRESS>
It works for me in Chrome 53.0.2785.89.
There's one caveat though. You cannot update configuration to a BW higher
than the one you set up initially with publish(stream, {maxVideoBW:}).
Give it a try and let me know if that is your case.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/ging/licode/issues/522#issuecomment-244903130, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AACgamGetJxIgj3c0-AV5gVXMUwyetJZks5qnTdKgaJpZM4J1lZw
.

--
Alvaro

@zevarito Yep, that should work.
Also, keep in mind you have to have a subscriber for all of this to take effect

ok, so I found the problem.
The subscriber can't be myself.
I was testing the function in a scenario where I was publishing my stream and I was also subscribing myself. In this scenario it won't work without sdp renegotiation. So, I simply added Sdp: true to options.
(changed in commit 6ea796a)
I also take this opportunity to ask you a question very important to me, that no one ever answered:
I made some modification to the externalOutput  to make it compatible with rtsp streaming that I'll happy to share with you guys, for a "broadcast" scenario where a third party transcoder will to the job to transcode vp8 + opus in some more scalable and suitable format (ex. WowzaMediaServer).
My problem is that if there's no one in the room, I'll only publish @ 300kbit, the quality is poor and with lots of artifacts (not suitable for broadcast) and I cannot understand why.
Whenever other users join, the bandwidth "unlocks" and everything is fine.
This forces me to self-subscribe when publishing and it kinda sucks cause there's no need in a broadcast to double the bandwidth only to make rembs and other kind of feedbacks mechanism take place.
I know that probably its due to feedbacks sent from the other party to chrome that unlocks the bandwidth, but: Is there any simple way to increase this default 300kb  to some higher level or, better, emulate these feedback directly from the server?
Thanks a lot

@kekkokk
It is what you suspect. Licode relays on feedback from subscribers to tell the publisher what is going on. Actually, the 300kbps is a Chrome thing.
We have tried a couple of, (admittedly hacky) mechanisms for your scenario without very satisfying results across different browsers. We will, however, allocate more time to the RTP/RTCP part in the short to mid term and that should solve your problem. We'll keep this scenario in mind, sorry it's awkward right now.


Adjust closed proposals sort order on the proposals page

This ticket https://github.com/vegaprotocol/frontend-monorepo/issues/3681
....could have been clearer.
The sort that has now been applied seems to have beenn applied across ALL proposals whether Open or Closed.  So it means that now to find the most recently closed proposals you have to scroll right to the bottom of the page.
We need different sort orders on Open vs Closed proposals.
Open ones leave as defined in that ticket.
Closed ones:

Enacted shoudl be sorted by enactmentTimestamp with most recent at the top
Rejected should be sorted by closingTimestampwith most recent at the top (since they dont have enacted)

Re-opening ticket as closed proposals are not sorted by enactmentTimestamp


Check IMAGETYP first before applying filtering

In the notebook on "Filtering out COS Data taken during the Day or Night", the user might wonder whether the observation they would like to analyze should remove time intervals with positive solar altitudes. Hence, maybe add some descriptions to check the fits header keyword IMAGETYP first somewhere in Section 1.1?

Hi @jmao2014 - are you talking about checking IMAGETYP to verify that the data was taken in TIME-TAG mode? I'm not entirely sure what you're getting at? Would you mind clarifying? Thank you!

Hi @nkerman, COS has two modes of data collection (TIME-TAG and ACCUM). In the exemplary notebook, the observation lbry01i6q is taken with the TIME-TAG mode. When dealing with other observations, one might want to check first whether the data is taken with the TIME-TAG mode. Does this make sense?


[Error] bolt-app failed to match all allowed scheme [jason-pointer:/view]

Description
Describe your issue here.
What type of issue is this? (place an x in one of the [ ])

[ ] bug
[ ] enhancement (feature request)
[x] question
[ ] documentation related
[ ] example code related
[ x] testing related
[x ] discussion

Requirements (place an x in each of the [ ])

[ ] I've read and understood the Contributing guidelines and have done my best effort to follow them.
[ ] I've read and agree to the Code of Conduct.
[ ] I've searched for any related issues and avoided creating a duplicate issue.

Bug Report
Filling out the following details about bugs will help us solve your issue sooner.
Reproducible in:
package version:
node version:
OS version(s):
Steps to reproduce:
1.create an array with some value
2.print it and delete all the elements in array
3.again print it
Expected result:
I need to open a dialog box or popup a notification that the all items are removed or empty
Actual result:
[Error] bolt-app failed to match all allowed scheme [jason-pointer:/view]
Attachments:

Hi @logesh-PLK, thanks for writing in!
We are unable to help you out with given information so far. The error message usually indicates the modal view JSON data structure is not valid (the view that you passed to views.open API call).
I would suggest using Block Kit Builder to check what's wrong with your JSON data.
I hope this helps!

I have a similar problem. At what sometimes fulfills and sometimes throws out an error.


Release v0.13.0

New feature

Onboard with options (#104)
Cross thing command trigger with optional data (#119)
Server code triggers with optional data (#136)

Improvement

retry on connection failure by PUT (#102)
Remove unnecessary public setter methods (#128)
Add javadoc for some method in Trigger class (#130)

Bug fix

Fix javadoc (#121)
Trigger#getTargetID() returns ID of ThingIFAPI#getTarget() (#112)

Internal

make request body from CommandForm. (#127)

Related PR: #144

released


fix(config): server restart on config dependencies changed on windows

Description
Server was not restarting when config dependencies was changed on windows.
This PR fixes this.
refs #3031
Additional context

What is the purpose of this pull request?

[x] Bug fix
[ ] New Feature
[ ] Documentation update
[ ] Other

Before submitting the PR, please make sure you do the following

[x] Read the Contributing Guidelines.
[x] Read the Pull Request Guidelines and follow the Commit Convention.
[x] Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
[x] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
[ ] Ideally, include relevant tests that fail without this PR but pass with it.

I think that we should directly normalize these paths when resolving the config. Check that we are already doing so for other paths.

I think that is better. I have changed it!


Persistent items

This adds a function called isPersistent to itemBase, which is checked for in sexEngine and inventory to avoid returning persistent items to the player at the end of sex scenes and to avoid clearing them at the start of sex scenes.
It also adds onSexStarted and onSexEnded to itemBase and calls them at the same time as the similarly named functions for effects and the like in baseCharacter.
These changes should make it easier for item logic to be run at the start and end of sex scenes, and for items to be given to NPCs permanently, which is useful for the Portal Expansion Mod that I'm making.
Hopefully, there are no edge cases where this code causes something to break.

Heya. Thankies. Little ask:
clearEquippedItems should stay as is, clearing all equipped items no matter what. You want to create a separate function and call it clearEquippedItemsKeepPersistent and replace the one in the resetEquipment with it
Are you sure items need onSexStart/End? Yeah, you said that you need it for the portal panties mod but why exactly. There is no better way?

Thank you


:wrench:  Addressed the issue of crashing when opening the app on the iPhone.

Issue

close #745

Overview (Required)

There was a case where an ImageBitmap was passed to the back/frontImage of a ShareableProfileCard with a size of 0, and if this was not handled, the application would always crash.
Therefore, we fixed it so that onCaptured is called when the Front/Back card to be captured is displayed and the size is definitely no longer 0.
Also, the share button could be pressed at any time, but in some cases it could be pressed before the image was generated, so this has also been addressed.

Links

https://developer.android.com/develop/ui/compose/graphics/draw/modifiers?hl=ja#drawwithcache

Movie (Optional)

I'm not sure, but the enabled = isReadyShare isn't applied in tests. 👀
enabled = isReadyShare

I'm waiting for linkDebugFrameworkIosArm64

Now there is no diffs. And I think I was able to open iOS's Compose Profile card

@takahirom
Thank you for correcting this in your sleep! 🙇


flat/flatMap missing - but how would I type a flat() function anyway?

Two parts to this question, because I came across the native JS function while writing my own (it's a common function in any case).
Stage 3 proposal:

https://tc39.github.io/proposal-flatMap/
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/flat

Typescript uses this monster definition - each possible level gets its own definition: https://github.com/Microsoft/TypeScript/blob/master/src/lib/esnext.array.d.ts
I came across the problem of having to type a flat() function after I added one to my own project:
const flat = function <T> (arr: Array<T | Array<T>>, depth?: number = 1): Array<T> {
   ...
};

This does not work. I cannot use just T, nor Array<T> (since at the very least I want to allow pure values as array items as well as arrays of pure values, for a nesting level of 1), nor does T | Array<T> seem to work even though I only have depth:1 arrays in my code.
Looking at the Microsoft definition of flat, this looks like a nightmare function to add types to. How would I do this in my code for my own function anyway? In the global definition which one could use something similar to what Typescript has, but when I document my own code and my own function inline I can only add one single type.
So another way to phrase this, when I don't write library definition types but type directly in the respective function, how can I have multiple different interfaces? interface in Flow seems to be for classes, I probably need different interfaces for the same function though (I don't have any classes in my functional-style code anyway).
I looked for an example how it was solved for libraries like Ramda which provide a flatten function, found this, and they use
declare function flatten<T>(x: T[]): T[];
declare function flatten(x: any[]): any[];

but the any route is not where I think this should end.
Related(?): https://github.com/facebook/flow/issues/4305, https://github.com/facebook/flow/issues/60

I also tried intersections as shown here: https://stackoverflow.com/a/45843913/544779
My intersection looks like this.
type FlattenFN =
    & (<T> (arr: T[], depth: 0, currentDepth?: number, result?:T[]) => T[])
    & (<T> (arr: Array<T | T[]>, depth: 1, currentDepth?: number, result?: T[]) => T[]);

but then I'm not sure about the generics parameter: Write type FlattenFN? But where do I get the type from when I actually use the intersection type:
const flatten: FlattenFN<???> = function (arr, depth, currentDepth = 0, result = []) {...}

Flow Try Link

I managed to get the easy cases to work using
type NestedArray<T> = Array<T | NestedArray<T>>;
function flatten<T>(xs: NestedArray<T>): Array<T> {
  ....
}

taken from here.
However, each time there is a promise somewhere it all fails. I'll have to investigate, I've seen that in many other experiments, always the Flow built-in promise definition caused problems.

I ended up using the definition from flow-typed for iodash.

I managed to get the easy cases to work using
type NestedArray<T> = Array<T | NestedArray<T>>;
function flatten<T>(xs: NestedArray<T>): Array<T> {
  ....
}

taken from here.
However, each time there is a promise somewhere it all fails. I'll have to investigate, I've seen that in many other experiments, always the Flow built-in promise definition caused problems.

How did this type work when it circularly references itself? I'm getting Type alias 'NestedArray circularly references itself' in TS playground.

in TS playground

TS? How about Flow Playground?

My two cents: for properly typing Array.prototype.flat there might be no other way than having one general case and then typing out all the special cases up to a certain depth.
For instance, flattening Array<Array<Array<string>>> with a depth of 2 should yield Array<string>, and I don't know a way of doing a generalized solution without having some advanced type-level programming.
(Feel free to correct me)

Is there no way to extend the core $ReadOnlyArray definition?
https://github.com/facebook/flow/blob/1e71f2358d5d989183aee8c8e9f97b68fa4577f4/lib/core.js#L196


benchmarks: make them build again

Thanks!
Could you extend CI so that it builds the benchmarks, so that we don't break them again in the future?


⚠ Bofrost.es has degraded performance

In fbbc9b6, Bofrost.es (https://www.bofrost.es) experienced degraded performance:

HTTP code: 200
Response time: 1491 ms

Resolved: Bofrost.es performance has improved in 368f8a1 after 6 minutes.


SetStatus filter does not override status on 504 response status from server

SetStatus filter doesn't override status when Timeout occurs in NettyRoutingFilter because response time exceeds httpclient's response-timeout.
Example configuration:
ok-http-response-service.ribbon.listOfServers: localhost:${wiremock.port}
spring:
  cloud:
    gateway:
      httpclient:
        connect-timeout: 100
        response-timeout: 400ms
      routes:
      - id: ok-http-response-service
      uri: lb://ok-http-response-service
      predicates:
      - Path=/ok-http-response-service/{somePath}
      filters:
      - SetStatus=200

Test to reproduce issue
    @Test
    public void shouldSendOkResponseForRequestThatTimedOut() {
        stubFor(get(urlEqualTo("/ok-http-response-service/request1"))
                .willReturn(serverError().withBody("timeout").withFixedDelay(401)));

        RestAssured.when()
                .get("/" + OK_HTTP_RESPONSE_SERVICE + "/request1")
                .prettyPeek()
                .then()
                .assertThat()
                .statusCode(OK.value())
                .body(containsString("Response took longer than timeout: PT0.4S"))
}

Same situation occurs on attempt to use custom override response/headers filter.
with any non-timeout response, filters work fine

Can you provide a complete, minimal, verifiable sample that reproduces the problem? It should be available as a GitHub (or similar) project or attached to this issue as a zip file.

Hi @ryanjbaxter,
I've created the project to reproduce the issue (attached here).
Hope you don't mind that I've added tests to the project which can be relative from my perspective to SetStatus filter.
I've mentioned in comments near each test the problem and my findings why it doesn't work.
For this specific issue relative test is:
SetStatusTest#shouldSendOkResponseForRequestThatTimedOut
Also, I've added tests for pre and post filter fail cases (where setStatus filter don't override result).
I will create a separate issue for it if you want, but I don't sure whether this is
"expected behavior"/bug/not implemented feature.
set-status.zip

FWIW I think that's normal behaviour.
If the gateway can't connect to its downstream service in a timely fashion it will respond with 504 Gateway Timeout.
The filter will only be executed after the chain has been successfully executed and a "next" signal is pushed back upstream.
Whenever something goes wrong - like a RuntimeException has been thrown - an "error" signal is pushed back upstream which isn't handled by the SetStatus filter.
Handling timeouts and such should be the responsibility of a circuit breaker instead of the SetStatus filter.

From the configuration perspective SetStatus doesn't give any clue when it will be used.
From the documentation:

In either case, the HTTP status of the response will be set to 401.

Handling timeouts and such should be the responsibility of a circuit breaker instead of the SetStatus filter.

IMO SetStatus filter is responsible for representation of the response not handling the exception. TimeoutException and all other types can still be handled by any exception handler (circuit breaker as well).

Hi @TYsewyn,
Firstly I agree with @Aloren, It was unexpected for me that SetStatus filter replaces Http status in some cases, and in others doesn't, as far as SetStatus is about to change the representation, but not the content(that's why I made a PR in the scope of #990).
I'll try to show a few use cases to express my expectation from the SetStatus filter.

The gateway should always return 200 OK (for example UI uses Flash or any other UI framework which can work with Http status 200 only).
In this case, the client doesn't care about the actual HTTP status (but it has to be 200 to process the response), since client takes data from response body/headers, and then do apply the circuit logic or any other based on it.

To build DSL based route, which will enrich (or replace) backend service response like:

spring:
  cloud:
    gateway:
      routes:
      - id: someService
      uri: lb://some-service
      predicates:
      - Path=/somePath/{somePath}
      filters:
      - DoResponseEnrich="basedOnThisParameter"
      - SetStatus=400

To replace response HTTP status to some other HttpStatus(defined via the contract with the client)  as an indicator of future changes in the route.

Hope my expectations about the filter are correct :)

To put that sentence back in context:
spring:
  cloud:
    gateway:
      routes:
      - id: setstatusstring_route
        uri: https://example.org
        filters:
        - SetStatus=BAD_REQUEST
      - id: setstatusint_route
        uri: https://example.org
        filters:
        - SetStatus=401

In either case, the HTTP status of the response will be set to 401.

Meaning: both filter configurations will work whenever you use a String (representing a valid HttpStatus)  or a number (eg. 401).
This filter will always do what you want if the gateway got a response from the downstream server.
That's exactly what you want with all of your bullet points: replace the status code of the response from the downstream server.
Whenever eg. a timeout occurs, there is a DNS resolving error or there is no available downstream instance, the gateway will go into an "error state" for the request meaning that there is no response and no way to automatically resolve this problem without the proper "knowledge" of what to do and so this will ALWAYS result in a 5xx error unless you add custom code to have this "manual" intervention.
And that's exactly where circuit breakers come into play!
Using circuit breakers you can define what needs to be done in case eg. you want to load balance over service X but your service registry can't provide you with any instance because they are all down. That's exactly what you are seeing here - you're downstream service isn't responding in a timely fashion - there is no response and so this can't be solved using only this filter.
I hope this clarifies when this filter will be executed.

Hi @TYsewyn,
Thanks for your answer.
So the purpose of this filter is not about format response for the client, but to format existing backend responses only?
And just to clarify that I've got your point.
In this case, the circuit breaker needs not just manage circuit, but also do the presentation formatting in the case service is unavailable, and also others "error signal cases" if they happen?
In the case I've got your point right I assume the behavior should be documented since spring cloud gateway depends on the circuit breaker logic.

Correct. It only changes the representation of the response.
If something goes wrong while requesting a response from the downstream server a gateway/reverse proxy will in most cases (if not always) return a 5xx response. eg. For a timeout we'll send a 504 Gateway Timeout, or in case the downstream server isn't behaving correctly we might send a 502 Bad Gateway. Therefor Spring Cloud Gateway isn't depending on any circuit breaker logic. This is normal behaviour for a gateway.
In case you don't want to return such a response to your client, you might want to look at the circuit breaker pattern. Spring Cloud Gateway provides a Hystrix gateway filter which will guard your services from cascading failures and it will also give you the possibility to have a fallback. This fallback mechanism will allow you to provide a default response for your client.
Keep in mind that Hystrix has entered maintenance mode and that this filter might/will be removed from Spring Cloud Gateway in the future.

@TYsewyn thanks,
I've flicked through [https://www.keycdn.com/support/nginx-rewrite-rules](NGINX documentation)
and [https://clouddocs.f5.com/api/irules/HTTP__respond.html](F5 docs)
From my experience, they behave a little bit differently(but to be 100% sure needed an additional test to break smth inside those applications and check the result).
Also, as far as I know, 5xx error compromises the server. So to avoid it I should also override DefaultErrorAttributes or even extend AbstractErrorWebExceptionHandler.
So the presentation layer should be duplicated in filters and Exception handlers to be sure that response will be rewritten always?
Hystrix is not suitable for me in the case because the filter may also raise an exception, and I have to guarantee the representation for the client.

since it seems that it isn't the feature which will be accepted in this project, at least for now.
I'll close this ticket


Missing 'to' in sentence

URL of the docs page
https://docs.dapr.io/developing-applications/sdks/dotnet/dotnet-development/
How is it currently worded?
However, developing microservices challenges you think about your local development process for more than one at a time.
How should it be worded?
However, developing microservices challenges you to think about your local development process for more than one at a time.
Screenshots
n.a.
Additional context
n.a.

/assign

Fixed


Improve link integrity popup volto17

backport of https://github.com/plone/volto/pull/6516/
fixed and improved link integrity popup when deleting item/items from folder contents.
Moved all code in a separate component 'ContentsDeleteModal'.
Added loader when loading references. If the call to @linkintegrity took a long time to load, it wasn't clear to the user that it was checking for broken references:

If only one item is selected to delete, it will display all the possibile broken references in a table, instead in a list. In the last column could be more than one reference:

If more than one item is selected to delete, it will not display the possibile broken references, but only the number, because it  will be a very long list (It worked like that before too):

📚 Documentation preview 📚: https://volto--6517.org.readthedocs.build/

@plone/volto-team approved, since it does not break the policy, since it applies to the CMSUI.


Release 0.4.43

The main feature in this release is a new API to write PAX extensions.

@alexcrichton Hmm I don't have publish rights on crates.io for this, mind doing that?

Done 👍


Fix freeze and topLeftCell in reader/writer

Fix #1175
Adds new methods set_pane_top_left_cell and set_sheetview_top_left_cell to zcl_excel_worksheet

Basic stuff. Tested for the 4 cases (nothing frozen, frozen rows only, frozen columns only, both rows and columns frozen), with Excel. No regression detected. I hope nobody minds if I merge the fix.


Preserve query and hash params when redirecting

Hi,
I noticed that if I type url http://mysite.com/docs?p=abc, hugo redirects me to http://mysite.com/docs/. This also happens for # params. Is there a way to preserve the query & hash params when this redirection happens?
Thanks.

Please use http://discuss.gohugo.io/ for questions/troubleshooting. Also see  Hugo Documentation.

I reposted to discuss. But I think it went to moderation queue.


'is int', 'is double' to be notified by compiler as platform specific operations

Operations 'is int', 'is double' to be notified by compiler as platform specific operations because behavior is completely different for web platform and others. It was strange decision to make this very basic thing different for web platform if target was to make Dart multiplatform language. Optimization has no relation to type detection logic - internally can use platform most effective representation.
int i = 0;
i is double == true // << Out of readability and any logic in any platform include JS
If decided to brake multiplatform and human logic compatibility - at least notify users to avoid unpredictable results in applications supposed to be really multiplatform.

It is using the platform's most efficient (and only) representation, Number, which is why the same value is used for both the integer 1 and the floating point number 1.
It's not possible to make a distinction at runtime because there is no distinction, there is only one value.
The logic is consistent, it's just that the object representing 1 is an instance of a class which implements both int and double.

We have a lint that might help: avoid_double_and_int_checks. It sounds like it will catch the kind of coding pattern you're concerned with.


Allow to specify Logging Domain

Add signed off by lines

This is a duplicate, it was already merged!


feat: implement the FileInput API for extra.FileInput

Hi!
I noticed that the FileInput from ipyvuetify.extra doesn't support all properties of v-file-input. In this PR I tried to implement most of the FileInput API while maintaining backward compatibility where possible. I suppose it closes #265.
I haven't found a beautiful way to pass v-file-input props to component without hardcoding the property names. I tried to filter keys of this.$data, but that will lead to possible issues with components derived from FileInput, such as solara.FileDrop. So I think this implementation will do for now, and the further task will be to combine it with the generated FileInput.
Please let me know your thoughts about this.

Also I made a reacton component for testing this code based on solara.FileDrop. If this PR gets merged, I can send a PR with something similar to solara repository.
import threading
from typing import Callable, TypedDict, BinaryIO, cast

import reacton
import solara

from ipyvuetify.extra.file_input import FileInput

class FileInfo(TypedDict):
    name: str
    size: int
    lastModified: int
    type: str
    file_obj: BinaryIO
    data: bytes | None

@reacton.component
def Upload(on_upload: Callable[[list[FileInfo]], None] = lambda _: None, lazy: bool = False, **kwargs):
    file_info, set_file_info = reacton.use_state(None)
    wired_files, set_wired_files = reacton.use_state(cast(list[FileInfo] | None, None))

    file_input = FileInput.element(
        outlined=True,
        counter=True,
        multiple=True,
        show_size=True,
        label="Files",
        placeholder="Select files...",
        chips=True,
        color="purple",
        on_v_model=set_file_info,
        disabled=bool(file_info),
        loading=bool(file_info),
        prepend_icon="mdi-upload",
        attributes={"accept": "image/*"},
        **kwargs
    )

    def wire_files():
        if not file_info:
            return

        real = cast(FileInput, reacton.get_widget(file_input))

        # workaround for @observe being cleared
        real.version += 1
        real.reset_stats()

        set_wired_files(cast(list[FileInfo], real.get_files()))

    reacton.use_effect(wire_files, [file_info])

    def handle_file(cancel: threading.Event):
        if not wired_files:
            return
        if on_upload:
            for wired_file in wired_files:
                wired_file["data"] = None if lazy else wired_file["file_obj"].read()
            on_upload(wired_files)
        set_file_info(None)

    result: solara.Result = solara.hooks.use_thread(handle_file, [wired_files])
    if result.error:
        raise result.error

    return file_input

if __name__ == "__main__":

    @reacton.component
    def Page():
        Upload(on_upload=print)

Yeah, this is great! I didn't know you could override a slot by adding it again. I also overlooked there was a slot for progress.
I agree with @maartenbreddels that we should not show warnings for the changed properties. In the notebook these will show up for the end-user. That end-user may not be the author and may not know how to fix it.
Looks like only show_progress is not backward compatible. It would be great if we could make that backward compatible as-well. Then this feature would have no breaking changes.
Note: I will force push to this branch to remove the erroneous commit that was temporarily on master.

I see there are lint-errors. This can be prevented by using pre-commit. I've just updated the readme with these instructions:
$ pip install -e ".[dev]"
$ pre-commit install

This will check and enforce code style when committing.
To update the existing code you can run:
$ pre-commit run --all-files

I've removed warnings and fixed code style issues.
Also, I returned the sync tag to file_info and used get_files() to set v_model value on file_info change. Now it is possible to remove the effect from the component above and use just on_v_model=set_wired_files.
I have a question: why this workaround is needed for solara.FileDrop? How observe can be cleared?
real.version += 1
real.reset_stats()

I added a computed property as a default loading state. If neither loading nor show_progress are set, the progress bar will appear automatically, but it is also easy to control the state manually.
I also thought about transforming v_model values into objects, that use pythonic key names (last_modified instead of lastModified) and maybe even allow attribute access (namedtuple?). It should be easier to do now, before this PR is merged. What do you think?

Sorry for the late response, I missed the notifications for the changes on this PR.

I added a computed property as a default loading state. If neither loading nor show_progress are set, the progress bar will appear automatically, but it is also easy to control the state manually.

Wouldn't it be better to leave out loading from the API, since it's controlled by the component internally?
The drag & drop functionality is really nice!, but it shouldn't be part of this PR. Are you able to move those commits to a new PR (that can be based on this PR)?

Hi, sorry to ask but is there any plan to merge this PR this at some point? Should it wait for a Vuetify3 upgrade?
Thanks ;)


✨ Add Smith[…]desc-thresh3_mask.nii.gz

Fixes

Related to #1 by @shnizzedy, @milham01 and @engfranco
Description

Adds a binary mask image at Z = 3 created from "10 well-matched maps from these, as shown in PNAS paper" from Correspondence between BrainMap and Resting-FMRI ICA components[^1]
[^1]: Smith, Stephen M et al. “Correspondence of the brain's functional architecture during activation and rest.” Proceedings of the National Academy of Sciences of the United States of America vol. 106,31 (2009): 13040-5. doi:10.1073/pnas.0905267106
Screenshots

https://user-images.githubusercontent.com/5974438/137552509-deec36d9-a3ed-4c70-854b-cc58f5250104.mp4
Checklist

[x] My pull request has a descriptive title (not a vague title like Update index.md).
[x] My pull request targets the main branch of the repository.
[x] My commit messages follow best practices.
[x] My code follows the established code style of the repository.
[ ] I added tests for the changes I made (if applicable).
[ ] I updated the changelog.
[x] I added or updated documentation (if applicable).
[ ] I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

I'm also not quite confident that I got the BIDS filename quite right:
Smith_space-MNI152NLin6_res-3x3x3_desc-thresh3_mask.nii.gz ?

I'm also not quite confident that I got the BIDS filename quite right:
Smith_space-MNI152NLin6_res-3x3x3_desc-thresh3_mask.nii.gz ?

As long as it gets pulled into the engine properly and the resulting output files are named properly, it's good.


api(metrics): Expose the allowOverride flag to the Metrics setGlobalProvider method

What happened?
Checking here:
https://github.com/open-telemetry/opentelemetry-js/blob/2dcc898514a60e5b9ac7ddea9b6f1e6c219097af/api/src/api/metrics.ts#L52
We can see that the allowOverride flag is not set, which then defaults to false.
https://github.com/open-telemetry/opentelemetry-js/blob/2dcc898514a60e5b9ac7ddea9b6f1e6c219097af/api/src/internal/global-utils.ts#L37
It is therefore not possible to re-register a global provider.
This causes some issues using when writing unit tests, when we'd like to start over (to avoid re-registering meters multiple times)
My current implementation of my tests is
const GLOBAL_OPENTELEMETRY_API_KEY = Symbol.for(`opentelemetry.js.api.1`);

beforeEach(() => {
  globalThis[GLOBAL_OPENTELEMETRY_API_KEY]['metrics'] = undefined; // Allow reset of metrics
  setupOtel();
});

Which is obtained by looking up the symbol in globalThis and resetting it. It will make the setup brittle, though.
Which is why it would be nice if allowOverride could be either set to true, or exposed to the user in the setGlobalMeterProvider method.
OpenTelemetry Setup Code
No response
package.json
No response
Relevant log output
No response

You could call disable() on provider in a afterEach() hook.

Sorry was a bit unclear. I mean api.metrics.disable() - see e.g. here

Works beautifully, thanks you and sorry for the noise :)