ModelicaStandardTables_*_read handling is fragile

Reported by hansolsson on 29 Jan 2016 16:05 UTC
The functions ModelicaStandardTables_*_read are somewhat fragile in some scenarios. If they are not called for the specific table after the constructor and before the *_getValue calls it will just silently return 0.0.
It would suggest calling *_read (with force=false) at the start of *_getValue, or possibly inline the test to make it more efficient.
I could add it myself if agreed on (would just be a few lines).
Another possibility would just be to generate an error message for this case. I find that less good, but still better than the current variant of silent failure is problematic.
I'm aware of #1103, and I agree that the possibility of re-reading the table is an argument for having it as two functions - but I don't like the implications in terms of error handling.

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

Comment by beutlich on 29 Jan 2016 22:59 UTC
I somehow understand the problem but I do not consider your proposed workaround as clean enough. It was already addressed in #1103 in such a way that dummy input tableAvailable was added to getTableValue to obtain some function call presedence. I wonder why it is considered fragile now and not 3 years ago? Does it help if the tableAvailable input is also passed to the C function *_getValue? If I remember correctly this was the way how the same issue was resolved for the new Noise functions quite recently. I also might add some additional assert or ModelicaError in *_getValue if the table data is missing.

Comment by hansolsson on 2 Feb 2016 11:20 UTC
Replying to [comment:1 beutlich]:

I somehow understand the problem but I do not consider your proposed workaround as clean enough. It was already addressed in #1103 in such a way that dummy input tableAvailable was added to getTableValue to obtain some function call presedence. I wonder why it is considered fragile now and not 3 years ago?

It was fragile then as well - I just didn't report it, partially because I hadn't fully understood the problem.
The solution can be as simple as (replacing error with some appropriate error message - or just removing that line):
double ModelicaStandardTables_CombiTable1D_getValue(void* _tableID, int iCol,
                                                    double u) {
    double y = 0.;
    CombiTable1D* tableID = (CombiTable1D*)_tableID;
    if (tableID != NULL && tableID->table == NULL) {
       /* Ensure that file is read */
       ModelicaStandardTables_CombiTable1D_read(_tableID, 0, 0);
       if (tableID->table == NULL) error
    }
    if (tableID != NULL && tableID->table != NULL && tableID->cols != NULL) {

Obviously for all tables.
If writing from scratch I might have used the coding practice of "NULL == ..." instead of "... == NULL".

Does it help if the tableAvailable input is also passed to the C function *_getValue?

Unfortunately no.
The problem is if tableAvailable is seen as correctly initialized (e.g. from another call) - but we haven't called the read-routine.

If I remember correctly this was the way how the same issue was resolved for the new Noise functions quite recently.

I realized that I had to check Noise as well - and that seems ok.

Comment by beutlich on 2 Feb 2016 11:32 UTC
Replying to [comment:2 hansolsson]:

Replying to [comment:1 beutlich]:

Does it help if the tableAvailable input is also passed to the C function *_getValue?

Unfortunately no.
The problem is if tableAvailable is seen as correctly initialized (e.g. from another call) - but we haven't called the read-routine.

That is what I do not understand. If tableAvailable is seen correctly initialized the table must have been read as expected. We have
initial algorithm
  tableOnFileRead := readTableData(tableID, false, verboseRead);
equation
  y = getTableValue(tableID, x, y, z, pre(z), tableOnFileRead);

thus getTableValue gets called after readTableData. Same approach as in Noise.

Modified by beutlich on 2 Feb 2016 11:37 UTC

Comment by beutlich on 2 Feb 2016 11:47 UTC
@hansolsson Please add or refer to a test model for easy reproduction.

Comment by beutlich on 2 Feb 2016 13:15 UTC

What do you mean by re-initialization of external objects? Calling the constructor again?
Does it help if readTableData is marked impure now? Thus impure functions in initial algorithms shall be called again. Is this specified?

Comment by beutlich on 2 Feb 2016 13:21 UTC

Does it help if reinit() was used?

Comment by hansolsson on 2 Feb 2016 13:51 UTC
Replying to [comment:7 beutlich]:

* What do you mean by re-initialization of external objects? Calling the constructor again?

Yes, in order to (re)start the simulation from known values.

* Does it help if readTableData is marked impure now? Thus impure functions in initial algorithms shall be called again. Is this specified?

That is a possibility, but I don't think it is a good idea.
I can see a case where we just log values to a file and the call in "initial algorithm" clears the file - and one doesn't want to repeat that.
Replying to [comment:8 beutlich]:

* Does it help if reinit() were used?

Don't think so - and that would require an extra state which seems problematic.
Is there a problem with adding the code I indicated above?
I could imagine a tiny issue with efficiency (duplicated null-pointer test), but even that could be avoided.

Comment by beutlich on 2 Feb 2016 14:24 UTC
Well, I summarize my concern about your proposed C code change again:
I consider it as not clean enough (aka hack) and am rather interested in a generic Modelica solution, that is w/o touching the C files but only the MO files. If you reconstruct an external object for a second simulation taking the initialization from a previous simulation you cannot assume that impure functions on that external object are still valid. This idea of re-initialization a simulation is completely new (at least for me if considering external objects) and should not be resolved en passant by such a hack.
Rather, we should properly discuss and specify how such a problem can be solved in a clean way. Note, there are more open issues on external objects that need to be (well at least) discussed.
Does it help if (protected) tableOnFileRead was not declared as fixed-false-parameter with initial algorithm but as variable instead with a declaration equation? Then it is no longer an initial state and saved in your initialization data. Not sure, if the newly added impure attribute then results in calling readTableData in each simulation step (which essentially matches your above C hack).

Comment by hansolsson on 2 Feb 2016 15:46 UTC
Replying to [comment:10 beutlich]:

Well, I summarize my concern about your proposed C code change again:
I consider it as not clean enough (aka hack) and am rather interested in a generic Modelica solution, that is w/o touching the C files but only the MO files.

To me the hack is the requirement of calling readTableData() in an initial algorithm instead of doing this in the constructor.
I view the initial sections are related to initializing the model - not to initializing the objects, and I recall that we at some time replaced "when initial() ..." by external objects in order to make that clearer (see the last paragraph below as well).
In general I prefer that the code is robust (i.e. automatically handling this case correctly) - and as a second option code that reports incorrect use (or sometimes a combination of the two - such that a warning is logged and then the code handles it).
Currently the code does neither - it detects a problem (null-pointer) and then silently produces a default result - instead of correcting it or reporting it.

Rather, we should properly discuss and specify how such a problem can be solved in a clean way. Note, there are more open issues on external objects that need to be (well at least) discussed.

I agree that we can consider having some special section for initializing external object - and also need to discuss other problems with them.

Does it help if (protected) tableOnFileRead was not declared as fixed-false-parameter with initial algorithm but as variable instead with a declaration equation? Then it is no longer an initial state and saved in your initialization data. Not sure, if the newly added impure attribute then results in calling readTableData in each simulation step (which essentially matches your above C hack).

I believe that would be an illegal call of impure function. If tableOnFileRead had been another external object instead it would likely have worked (I haven't tested).

Comment by beutlich on 2 Feb 2016 16:06 UTC
Replying to [comment:11 hansolsson]:

Replying to [comment:10 beutlich]:

Well, I summarize my concern about your proposed C code change again:
I consider it as not clean enough (aka hack) and am rather interested in a generic Modelica solution, that is w/o touching the C files but only the MO files.

To me the hack is the requirement of calling readTableData() in an initial algorithm instead of doing this in the constructor.

I guess you read the motivation in #1103.

I view the initial sections are related to initializing the model - not to initializing the objects, and I recall that we at some time replaced "when initial() ..." by external objects in order to make that clearer (see the last paragraph below as well).

I can't follow here since I am not aware of any other external objects in the MSL. Please give an example where "when initial()" was replaced by an external object.

In general I prefer that the code is robust (i.e. automatically handling this case correctly) - and as a second option code that reports incorrect use (or sometimes a combination of the two - such that a warning is logged and then the code handles it).
Currently the code does neither - it detects a problem (null-pointer) and then silently produces a default result - instead of correcting it or reporting it.

I stated already in comment:1 that adding assert or ModelicaError might be the preferable solution.

Rather, we should properly discuss and specify how such a problem can be solved in a clean way. Note, there are more open issues on external objects that need to be (well at least) discussed.

I agree that we can consider having some special section for initializing external object - and also need to discuss other problems with them.

Yes, please.

Does it help if (protected) tableOnFileRead was not declared as fixed-false-parameter with initial algorithm but as variable instead with a declaration equation? Then it is no longer an initial state and saved in your initialization data. Not sure, if the newly added impure attribute then results in calling readTableData in each simulation step (which essentially matches your above C hack).

I believe that would be an illegal call of impure function. If tableOnFileRead had been another external object instead it would likely have worked (I haven't tested).

This might look like an overkill especially if the table data comes from the model. If it however turns out that having tableOnFileRead as another external object is the clean solution than we are one step further. But I am not sure at all as of now. If this also changes the C API of ModelicaStandardTables we need to re-schedule it for the next MSL anyway (where I already planned to change the API).

Comment by beutlich on 2 Feb 2016 19:14 UTC
Replying to [comment:11 hansolsson]:

To me the hack is the requirement of calling readTableData() in an initial algorithm instead of doing this in the constructor.

If this is now considered as a hack I propose the following list of action items and hope we can discuss and agree on them.

Create a new ticket about clarification of initialization of external objects. It should be stated that external objects need solely to be initialized in their constructor, and especially must not be initialized by initial algorithms or initial equations. -> Hans
Based on that ticket, discuss external objects at one of the next MDMs and update the MLS (sooner or later). -> MD
Ask MAPLib project leader if to the keep milestone of this ticket (#1899) or re-schedule for MSL 3.3. -> Toni
For ModelicaStandardTables -> /me
Read table during construction of external object (with force=0). (As already was mentioned in #1103.)
Remove explicit call to readTableData and initial algorithm in MO files. (As already was mentioned in #1103.)
Get rid of dummy input tableAvailable from functions getTableValue, getTableValueNoDer, getDerTableValue and getNextTimeEvent. Since this breaks the backward compatibility it should be re-scheduled for the next major MSL 3.3.
Remove function readTableData from ModelicaTest.Tables.CombiTable2D.
Throw an error in C functions _getValue if table data is not available.
Open issue: What to do with getTableTimeTmin/getTableTimeTmax and t_min/t_max of CombiTimeTable? They are pure functions and fixed-false-parameters with initial algorithms, respectively, that depend on the external object.

Comment by beutlich on 3 Feb 2016 09:01 UTC
After speaking with Martin Otter we spotted two more open issues, esp. for the CombiTimeTable.
7. The current implementation with calling readTableData (with force=false) in the initial algorithm prevents repeated reading of the table data. Assume that reading the table data is an expensive call, then, when moving it to the constructor we get in performance trouble if the constructor/destructor functions are called (in pairs) more than once.
8. For the CombiTimeTable we call getNextTimeEvent in a when initial() statement. The first call of getNextTimeEvent has side effects on the external object because all the time events are determined and internally stored. Hence this is similar to the readTableData function and should also be avoided (if we forbid impure functions on external objects during initialization).
We also noticed that the same re-initialization problem might also be apparent for imported FMUs.

Comment by hansolsson on 5 Feb 2016 17:14 UTC
Replying to [comment:14 beutlich]:

After speaking with Martin Otter we spotted two more open issues, esp. for the CombiTimeTable.
7. The current implementation with calling readTableData (with force=false) in the initial algorithm prevents repeated reading of the table data. Assume that reading the table data is an expensive call, then, when moving it to the constructor we get in performance trouble if the constructor/destructor functions are called (in pairs) more than once.

In case we consider that an important case - and the external object isn't actually used - I would delay loading the table until the first time the table is needed (i.e. proposed variant of getValue - but readTableData is then just an internal function in the C-files).
However, I find it odd that the constructors are called and nothing is used for them.

For the CombiTimeTable we call getNextTimeEvent in a when initial() statement. The first call of getNextTimeEvent has side effects on the external object because all the time events are determined and internally stored. Hence this is similar to the readTableData function and should also be avoided (if we forbid impure functions on external objects during initialization).

If it is done any time "getNextTimeEvent" is called the first time it is just caching to me and should be safe - but we could move this to the constructor as well.

We also noticed that the same re-initialization problem might also be apparent for imported FMUs.

Comment by hansolsson on 5 Feb 2016 17:18 UTC
Replying to [comment:13 beutlich]:

Replying to [comment:11 hansolsson]:

To me the hack is the requirement of calling readTableData() in an initial algorithm instead of doing this in the constructor.
If this is now considered as a hack I propose the following list of action items and hope we can discuss and agree on them.

Create a new ticket about clarification of initialization of external objects. It should be stated that external objects need solely to be initialized in their constructor, and especially must not be initialized by initial algorithms or initial equations. -> Hans

Ok, will do.

Open issue: What to do with getTableTimeTmin/getTableTimeTmax and t_min/t_max of CombiTimeTable? They are pure functions and fixed-false-parameters with initial algorithms, respectively, that depend on the external object.

As I see it that isn't a problem. Similarly as if one called ...getValue in an inital algorithm to perform a table-lookup - it doesn't change the external object; it just stores a value.

Comment by otter on 18 Feb 2016 16:01 UTC
There seems to be the following options, and we should agree on one of the two:
Constructor reads table
For Blocks.Tables.XXX, the code would be:
protected
  // The table is read from file
  Types.ExternalCombiTable1D tableID = Types.ExternalCombiTable1D(...);

equation
  ...
  // Interpolate u[i] in the table
  y[i] = getTableValue(tableID, i, u[i], 1)

It is assumed that a Modelica tool calls the constructor only once. If this is not the case, the reading of large tables is possible but will become unnecessarily unefficient.
For Blocks.Sources.CombiTimeTable, the code would be:
protected
  // The table is read from file
  Types.ExternalCombiTimeTable1D tableID = Types.ExternalCombiTimeTable1D(...);

initial algorithm
  // Get t_min, t_max
  t_minScaled := getTableTimeTmin(tableID, tableOnFileRead);
  t_maxScaled := getTableTimeTmax(tableID, tableOnFileRead);
  t_min := t_minScaled*timeScale;
  t_max := t_maxScaled*timeScale;
equation
  ...
  when {time >= pre(nextTimeEvent),initial()} then
    nextTimeEventScaled = getNextTimeEvent(tableID, timeScaled, tableOnFileRead);
    if (nextTimeEventScaled < DBL_MAX) then
      nextTimeEvent = nextTimeEventScaled*timeScale;
    else
      nextTimeEvent = DBL_MAX;
    end if;
  end when;

  ...
  // Interpolate u[i] in the table
  y[i] = p_offset[i] + getTableValue(tableID, i, timeScaled, nextTimeEventScaled, pre(nextTimeEventScaled), 1);

In the documentation it is stated, that this table model assumes that after initialization, the table on file is not changed. For example assume that the simulation is terminated at 2 s and the final state stored on file (e.g. to find a steady-state). The next day the simulation is continued started at 2 s and reading the final state at 2 s and starting from this final state. Then, the simulation might fail, if the table on file has been changed when restarting at 2 s (e.g. might be the case if pre(nextTimeEventScaled) is no longer consistent, and/or t_minScaled/t_maxScaled has changed and is used in the user model).
First access reads table
For Blocks.Tables.XXX, the code would be:
protected
  // The tableID is initialized (a memory for a struct is allocated where the input
  // arguments of the constructor are stored; the address of this struct is returned;)
  Types.ExternalCombiTable1D tableID = Types.ExternalCombiTable1D(...);

equation
  ...
  // Interpolate u[i] in the table (if the table is not yet read, it is first read from file
  // and a pointer is stored in the tableID struct, before interpolation takes place)
  y[i] = getTableValue(tableID, i, u[i], 1)

This table reading is always efficient because the table is only read once.
For Blocks.Sources.CombiTimeTable, the code would be:
protected
  // The tableID is initialized (a memory for a struct is allocated where the input
  // arguments of the constructor are stored; the address of this struct is returned;)
  Types.ExternalCombiTimeTable1D tableID = Types.ExternalCombiTimeTable1D(...);

initial algorithm
  // Get t_min, t_max
  t_minScaled := getTableTimeTmin(tableID, tableOnFileRead);
  t_maxScaled := getTableTimeTmax(tableID, tableOnFileRead);
  t_min := t_minScaled*timeScale;
  t_max := t_maxScaled*timeScale;
equation
  ...
  when {time >= pre(nextTimeEvent), initial()} then
    nextTimeEventScaled = getNextTimeEvent(tableID, timeScaled, tableOnFileRead);
    if (nextTimeEventScaled < DBL_MAX) then
      nextTimeEvent = nextTimeEventScaled*timeScale;
    else
      nextTimeEvent = DBL_MAX;
    end if;
  end when;

  ...
  // Interpolate u[i] in the table (if the table is not yet read, it is first read from file
  // and a pointer is stored in the tableID struct, before interpolation takes place)
  y[i] = p_offset[i] + getTableValue(tableID, i, timeScaled, nextTimeEventScaled, pre(nextTimeEventScaled), 1);

Again, in the documentation it is stated, that this table model assumes that after initialization, the table on file is not changed. For example assume that the simulation is terminated at 2 s and the final state stored on file (e.g. to find a steady-state). The next day the simulation is continued started at 2 s and reading the final state at 2 s and starting from this final state. Then, the simulation might fail, if the table on file has been changed when restarting at 2 s (e.g. might be the case if pre(nextTimeEventScaled) is no longer consistent, and/or t_minScaled/t_maxScaled has changed and is used in the user model).
There seems to be no essential difference for the two variants and therefore it is probably cleaner to use variant 1.
Note, there is no way with the current design that CombiTimeTable works correctly, if the table is modified before re-reading it af the previous final state, because t_min/t_max are parameters and are read from the final state, and becuase pre(nextTimeEvent) is also read from the final state and cannot be set to a correct value when the simulation is restarted.
Note, we should also simplify the MSL, by introducing the package "Blocks.Tables.Internal" which shall contain all external function calls to the tables, in order that these definitions do not have to be duplicated several times. I can do this, if you (Thomas) agree.

Comment by beutlich on 18 Feb 2016 19:20 UTC
Shouldn't we remove the initial algorithm completely, i.e. have
    final parameter Modelica.SIunits.Time t_min=t_minScaled*timeScale
      "Minimum abscissa value defined in table";
    final parameter Modelica.SIunits.Time t_max=t_maxScaled*timeScale
      "Maximum abscissa value defined in table";
    final parameter Real t_minScaled=getTableTimeTmin(tableID, tableOnFileRead)
      "Minimum (scaled) abscissa value defined in table";
    final parameter Real t_maxScaled=getTableTimeTmax(tableID, tableOnFileRead)
      "Maximum (scaled) abscissa value defined in table";

instead of
    final parameter Modelica.SIunits.Time t_min(fixed=false)
      "Minimum abscissa value defined in table";
    final parameter Modelica.SIunits.Time t_max(fixed=false)
      "Maximum abscissa value defined in table";
    final parameter Real t_minScaled(fixed=false)
      "Minimum (scaled) abscissa value defined in table";
    final parameter Real t_maxScaled(fixed=false)
      "Maximum (scaled) abscissa value defined in table";
/*...*/
  initial algorithm
    t_minScaled := getTableTimeTmin(tableID, tableOnFileRead);
    t_maxScaled := getTableTimeTmax(tableID, tableOnFileRead);
    t_min := t_minScaled*timeScale;
    t_max := t_maxScaled*timeScale;

as otherwise a reinitialized simulation relies on wrong t_min/t_max from the previous simulation and the strong requirement that the external table file must not change?
Since tableOnFileRead becomes obsolete now it would be smarter to postpone this issue to the next MSL update where I am allowed to remove it.

Comment by hansolsson on 19 Feb 2016 08:32 UTC
Replying to [comment:18 beutlich]:

Shouldn't we remove the initial algorithm completely, i.e. have

I agree that it seems logical, but would delay it to the next release.

Comment by otter on 19 Feb 2016 08:46 UTC
Replying to [comment:18 beutlich]:

Shouldn't we remove the initial algorithm completely, i.e. have
    final parameter Modelica.SIunits.Time t_min=t_minScaled\*timeScale
      "Minimum abscissa value defined in table";
    final parameter Modelica.SIunits.Time t_max=t_maxScaled\*timeScale
      "Maximum abscissa value defined in table";
    final parameter Real t_minScaled=getTableTimeTmin(tableID, tableOnFileRead)
      "Minimum (scaled) abscissa value defined in table";
    final parameter Real t_maxScaled=getTableTimeTmax(tableID, tableOnFileRead)
      "Maximum (scaled) abscissa value defined in table";

instead of
    final parameter Modelica.SIunits.Time t_min(fixed=false)
      "Minimum abscissa value defined in table";
    final parameter Modelica.SIunits.Time t_max(fixed=false)
      "Maximum abscissa value defined in table";
    final parameter Real t_minScaled(fixed=false)
      "Minimum (scaled) abscissa value defined in table";
    final parameter Real t_maxScaled(fixed=false)
      "Maximum (scaled) abscissa value defined in table";
/\*...\*/
  initial algorithm
    t_minScaled := getTableTimeTmin(tableID, tableOnFileRead);
    t_maxScaled := getTableTimeTmax(tableID, tableOnFileRead);
    t_min := t_minScaled\*timeScale;
    t_max := t_maxScaled\*timeScale;

as otherwise a reinitialized simulation relies on wrong t_min/t_max from the previous simulation and the strong requirement that the external table file must not change?

Yes, you are completely right (remove initial algorithm section and call the function in the parameter declaration).

Since tableOnFileRead becomes obsolete now it would be smarter to postpone this issue to the next MSL update where I am allowed to remove it.

Hm. Do you mean to keep the tables as they are now, and do not perform the changes above?
The trouble is that no one knows when an MSL version will be released that is no longer backwards compatible (of course we try to keep MSL backwards compatible as long as possible). Therefore, we should not wait until this event will happen because with luck this will be in 5-10 years. The table functions are anyway not really critical, because until now they are only interfaced in a protected section and are not directly accessible.
Ths simplest solution is just to keep all interfaces, and also keep function tableOnFileRead (if this function is used by a user, the only drawback is that the file is read twice; but it still works).

Comment by anonymous on 19 Feb 2016 08:53 UTC
Replying to [comment:6 hansolsson]:

External objects must be re-initialized in that case (since we don't have a save/restore interface) - but we shouldn't initialize everything according to the initial algorithms.

So introduce save/restore to specification 3.4 instead, and allow its use in MSL 3.4. If Modelica does not support save/restore it cannot be important to fix it in the current MSL and current specification.

Comment by otter on 19 Feb 2016 08:58 UTC
Replying to [comment:21 anonymous]:

Replying to [comment:6 hansolsson]:

External objects must be re-initialized in that case (since we don't have a save/restore interface) - but we shouldn't initialize everything according to the initial algorithms.

So introduce save/restore to specification 3.4 instead, and allow its use in MSL 3.4. If Modelica does not support save/restore it cannot be important to fix it in the current MSL and current specification.

This is a timing issue: Releasing Modelica 3.4 takes time, until all the Modelica tools supports it takes A LOT OF TIME (only Dymola supports 3.3, and 3.3 was released 2012), and only afterwards one could utilize this feature in MSL. To summarize, it is out of the question to wait for it.
With the slight changes above, it will be possible that a model containing tables can be restarted from a previous simulation run and for some end-users this is important (independent of the tool)

Comment by hansolsson on 19 Feb 2016 09:01 UTC
Replying to [comment:21 anonymous]:

Replying to [comment:6 hansolsson]:

External objects must be re-initialized in that case (since we don't have a save/restore interface) - but we shouldn't initialize everything according to the initial algorithms.

So introduce save/restore to specification 3.4 instead, and allow its use in MSL 3.4. If Modelica does not support save/restore it cannot be important to fix it in the current MSL and current specification.

Save/restore is important for external objects that have an internal state that influences the simulation, and we need to work on that since there are important applications.
However, tables only have an internal state for efficiency reasons - it shouldn't impact the result (as far as I recall - there might be some minor exception).
In theory we could skip the table-object and just read the table file for each table-lookup - and we would get the same result, just slower.

Comment by otter on 19 Feb 2016 09:06 UTC
If the issue is "too much changes to the table implementation" now, we could also follow the original suggestion by Hans and only make a  minor change: If the table is not there when interpolating, it is just read. As I recall, this would be only a one-line change in the table (I and also do it a and test it, if timing is the issue)

Comment by otter on 19 Feb 2016 14:36 UTC
Yet another possibility is to keep all table C-functions unchanged, but

Rename the constructor C-function to ModelicaStandardTables_CombiTimeTable_initWithReadFile (this function just calls the existing functions to init and read the file).
Change the Modelica code according to the above suggestions (I talked to Hans, and it is fine for him to initialize the T_min/T_max parameters in the declaration as suggested above).

This looks to me like a simple, clean, and backwards compatible solution.

Comment by otter on 19 Feb 2016 15:33 UTC
As proposed, moved external C-function definitions of tables out of the protected sections in a separate package (Blocks.Tables.Internal) in b6c30ee8ce312c83ab48bbc89a9bdd507481793b.
Advantages:

No double definitions anymore (and therefore easier to maintain).
Code for the table models is simpler (because function definitions no longer present in the protectec section).
The functions can also be called in another context (e.g. for scripting).
Tested the changes with "Check" and "Check with Simulations".

Comment by otter on 22 Feb 2016 16:41 UTC
The proposed changes are in the attached patch-file. Thomas, please apply this patch and re-generate the table object libraries. I have tested with ModelicaTest (by including the new C-code with an Include; after the test, I have removed the Include directive and included the code in ModelicaStandardTables.c).

Comment by beutlich on 24 Feb 2016 13:54 UTC
Two remarks:

You changed the type of the external object tableID to parameter? Was this by design? I guess it is not needed in the context of this ticket.
The constructor of the external object now takes the parameter verboseRead as input argument. However, verboseRead is disabled (in the dialog) if the table is not to be read from file. Thus, it is not a clean implementation. In the current implementation verboseRead is only accessed if tableOnFile is true.

Comment by otter on 24 Feb 2016 14:12 UTC
Replying to [comment:28 beutlich]:

Two remarks:
* You changed the type of the external object tableID to parameter? Was this by design? I guess it is not needed in the context of this ticket.

You are right. It does not matter, and it is probably safer to not use parameter (so please, remove it).

* The constructor of the external object now takes the parameter verboseRead as input argument. However, verboseRead is disabled (in the dialog) if the table is not to be read from file. Thus, it is not a clean implementation. In the current implementation verboseRead is only accessed if tableOnFile is true.

This approach is used in many Modelica models and in many (non-Modelica related) libraries: Some parameters only have an effect if some other parameters have a particular value (otherwise their value is ignored). Just take the LAPACK library, where this is used everywhere. In Modelica this is anyway just a user-interface issue define via the Dialog annotation (so that a tool can lock a dialog field). Still, the value can still be changed in the text layer (even if the parameter is not used).

Comment by beutlich on 24 Feb 2016 14:18 UTC
Replying to [comment:29 otter]:

Replying to [comment:28 beutlich]:

Two remarks:
* You changed the type of the external object tableID to parameter? Was this by design? I guess it is not needed in the context of this ticket.

You are right. It does not matter, and it is probably safer to not use parameter (so please, remove it).

OK.

* The constructor of the external object now takes the parameter verboseRead as input argument. However, verboseRead is disabled (in the dialog) if the table is not to be read from file. Thus, it is not a clean implementation. In the current implementation verboseRead is only accessed if tableOnFile is true.

This approach is used in many Modelica models and in many (non-Modelica related) libraries: Some parameters only have an effect if some other parameters have a particular value (otherwise their value is ignored). Just take the LAPACK library, where this is used everywhere. In Modelica this is anyway just a user-interface issue define via the Dialog annotation (so that a tool can lock a dialog field). Still, the value can still be changed in the text layer (even if the parameter is not used).

So no need to have it like this: if tableOnFile then verboseRead else false

Comment by otter on 24 Feb 2016 14:23 UTC
Replying to [comment:30 beutlich]:

Replying to [comment:29 otter]:

Replying to [comment:28 beutlich]:

Two remarks:
* You changed the type of the external object tableID to parameter? Was this by design? I guess it is not needed in the context of this ticket.

You are right. It does not matter, and it is probably safer to not use parameter (so please, remove it).
OK.

* The constructor of the external object now takes the parameter verboseRead as input argument. However, verboseRead is disabled (in the dialog) if the table is not to be read from file. Thus, it is not a clean implementation. In the current implementation verboseRead is only accessed if tableOnFile is true.

This approach is used in many Modelica models and in many (non-Modelica related) libraries: Some parameters only have an effect if some other parameters have a particular value (otherwise their value is ignored). Just take the LAPACK library, where this is used everywhere. In Modelica this is anyway just a user-interface issue define via the Dialog annotation (so that a tool can lock a dialog field). Still, the value can still be changed in the text layer (even if the parameter is not used).
So no need to have it like this: if tableOnFile then verboseRead else false

Hm. Do you mean to have "if tableOnFile then verboseRead else false" as actual argument for verboseRead to the C-function (constructor)? Feel free to use either solution. Providing directly parameter "verboseRead" is also fine (since it does not have an effect).

Modified by beutlich on 25 Feb 2016 11:26 UTC

Comment by beutlich on 25 Feb 2016 12:21 UTC
One more problem with the proposed initWithRead shortcut constructors is, that they leak memory if reading of the table fails, i.e. ModelicaError is called in the read function or its called functions.

Comment by otter on 25 Feb 2016 13:20 UTC
Thomas, since it seems that you do not have time to fix this, I will do it tomorrow morning (based on the proposal by Hans).

Comment by otter on 25 Feb 2016 13:22 UTC
Replying to [comment:34 otter]:

Thomas, since it seems that you do not have time to fix this, I will do it tomorrow morning (based on the proposal by Hans).

Forgot: This includes regeneration of the table object libraries (so you do not have to do anything).

Comment by beutlich on 26 Feb 2016 07:34 UTC
For the record: #1907 is the corresponding MLS ticket on improvement of the initialization of external objects.

Comment by otter on 26 Feb 2016 09:38 UTC
Replying to [comment:30 beutlich]:

Replying to [comment:29 otter]:

Replying to [comment:28 beutlich]:

Two remarks:
* You changed the type of the external object tableID to parameter? Was this by design? I guess it is not needed in the context of this ticket.

You are right. It does not matter, and it is probably safer to not use parameter (so please, remove it).
OK.

I recognized that this is not possible in Dymola (and most likely this is also the Modelica Specification interpretation). The reason is:
  final parameter Real t_minScaled = Internal.getTimeTableTmin(tableID);
  final parameter Modelica.SIunits.Time t_min = t_minScaled*timeScale;
protected
  parameter Modelica.Blocks.Types.ExternalCombiTimeTable tableID = ...

tableID is used in the parameter expression t_minScaled, which is in turn used in the parameter expression t_min. According to the Modelica Specification this means that all variables referenced in the parameter expression must be parameters or constants. Without the "parameter" prefix for tableID, Dymola treats tableID not as parameter and triggers an error (when the "parameter" prefix is not present. I therefore keep this implementation.

Comment by otter on 26 Feb 2016 12:06 UTC
Fixed in f85205a60371a2ebd01d47d2232808e9b814c85b along the lines of the discussion above (new constructor functions that initialize tableID and read the file in the constructor). Checked with Dymola 2017 Alpha for win64 by running Check and CheckWithSimulation for ModelicaTest.Tables.

Changelog modified by otter on 26 Feb 2016 12:06 UTC
Initialization of Blocks.Tables.XXX and Blocks.Sources.CombiTimeTable changed, so that the tables are read from file in the constructor. It is then possible to stop a simulation and continue simulation also if tables are contained in the model

Comment by beutlich on 29 Feb 2016 22:14 UTC
I'd like to

rename _initWithRead to init2
add the missing extrapolation parameter as a stub to the 1D and 2D variants in preparation for #1839
fix the memory leaks if the constructor fails in read, isValidTable or getTableSource.

However, the third action item requires some thorough rework and test which I only can do during spare-time.

Comment by otter on 1 Mar 2016 07:18 UTC
Replying to [comment:39 beutlich]:

I'd like to

rename _initWithRead to init2

The "old name" is better, since clearer. Please, keep it.

add the missing extrapolation parameter as a stub to the 1D and 2D variants in preparation for #1839

And a stub for "instance name", and a stub for uniform-grid, and a stub for an interpolation method that requires additional options, ...
Please, don't do this now. New functions should be introduced, when there is a design for it and by experience the chance is high that the design is wrong (just by guessing, if you don't implement and test it).

fix the memory leaks if the constructor fails in read, isValidTable or getTableSource.

Explain why there should be a memory leak (if there is one, it was already present in the previous release). Note, the current implementation is
void* ModelicaStandardTables_CombiTable1D_initWithRead(...) {
   void* tableID = ModelicaStandardTables_CombiTable1D_init(...);
   double success;
   if ( tableID == NULL ) return NULL;
   success = ModelicaStandardTables_CombiTable1D_read(tableID, 0, verbose);
   if ( success == 0.0 ) {
      ModelicaStandardTables_CombiTable1D_close(tableID);
      return NULL;
   }
   return tableID;
}

Note, tableID <> NULL is only returned if memory is successfully allocated.
If xxx_CombiTable1D_read(..) has an error during reading, it should return "success = 0.0" according to the documentation, but then the potentially allocated memory is freed.

Comment by beutlich on 2 Mar 2016 12:31 UTC
Reopen now and give detailed reasons later, i.e., in my Modelica dedicated spare-time. Cannot afford to write hour-long reasoning during office hours.

Comment by beutlich on 2 Mar 2016 12:32 UTC
If I accept a ticket it means I am going to work on it. So please do not close it instantly.

Comment by beutlich on 2 Mar 2016 17:25 UTC
This is why I have concerns with the current solution:

Both in comment 3 of #1820 and comment 11 of #1839 I stated that I do not want to introduce new C-API functions in ModelicaStandardTables.h. Whereas #1820 would need an additional verbose parameter for the constructors, #1839 requires a new extrapolation parameter (also for the constructors). By purpose, I decided to not touch the C-API and in case of #1839 reschedule the ticket milestone.
With this ticket (#1899) a new use case came around, which is the reinitialization of a saved simulation with external objects. Note that since #1103 there was not a single issue with the existing ModelicaStandardTables regarding reading tables from files outside the constructor but in an extra function called during an initial algorithm section. I understand the new problem, but see, we needed to touch the MLS for it (#1907).
Reinitialization of a saved simulation with external objects is a completely new topic. This ticket (#1899) showed some ideas/questions if we shall add new API functions to ExternalObject which make save and restore easier.
If we fix it now as hack by introducing new C-API functions (for the verbose parameter of the constructor) though neither #1907 nor the ExternalObject is properly dicussed we run the risk of adding stuff that is hard to get undone later.

New functions should be introduced, when there is a design for it and by experience the chance is high that the design is wrong (just by guessing, if you don't implement and test it).

You are right here. If there is neither a design nor a proper discusssion on it we should not add new C-API functions _initWithRead. Despite the missing design, the name _initWithRead is also misleading if the table data is not read from file but taken from the model. _initWithReadIfTableOnFile does not really sound better, thus my generic proposal _init2.
About the unlucky memory leak: Before f85205a60371a2ebd01d47d2232808e9b814c85b the following flow of functions worked:

Call constructor w/o reading table data
Try to read table data from file
Reading table data failed as e.g., MAT file format v7.3 is not supported by the tool. Read function fails with ModelicaError
Call destructor to free all allocated memory

After f85205a60371a2ebd01d47d2232808e9b814c85b we now always have:

Call constructor and try to read table data from file
Reading table data failed as e.g., MAT file format v7.3 is not supported by the tool. Read function fails with ModelicaError and does not return to the calling constructor and leakes resources.
No need/way to call destructor

You see, it was a design decision to not call the read function in the constructor. If this design decision is now reverted (by validity of #1907) we also need to update the C code properly. The current C implementation punishes the hack of f85205a60371a2ebd01d47d2232808e9b814c85b unexpectedly but dramatically. For sure it can and needs to be solved in an clean way.
Please note, that SimulationX always runs shared objects for the simulation which is the reason I do care a lot for resource leaks. In case of Dymola, memory leaks are not relevant as long as the executable binary runs in an own process.

My proposal:

Revert f85205a60371a2ebd01d47d2232808e9b814c85b and keep particularly ModelicaStandardTables.h untouched
Rethink of a proper solution of how to deal with the reinitialization of external objects (not yet considered as part of #1907). Maybe DS Lund can present what they are actually doing with some more technical details.
Reschedule this ticket (#1899) to the next MSL
Solve it in a really clean way along with other relevant table proposals (from #1153)

Since I feel this is a very important issue the MAP-LIB members should vote on this (or an alternative) proposal.

Comment by otter on 3 Mar 2016 04:58 UTC
Reverted

f85205a60371a2ebd01d47d2232808e9b814c85b, so tables are again read in the initial section and not in the constructor.
b6c30ee8ce312c83ab48bbc89a9bdd507481793b, so table functions are again protected functions in the model and cannot be accessed, say, from scripting. This reversion is necessary because otherwise functions would be present in MSL that do not return in case of error.

These reversions have been tested with Dymola 2017 Beta.1 (Check and CheckWithSimulation on Modelica and ModelicaTest).
Opened a branch in r9121 (https://svn.modelica.org/projects/Modelica/branches/development/ModelicaStandardTables-Fix1899), in which the issue is fixed with the suggestion from Hans (= if table is not present in xx_getValue(..), then the table is again read from file). This is discussed in the separate ticket #1941. A tool vendor might use the ModelicaStandardTables.lib/.a file from this branch in order to get an immediate fix for the bug reported in #1899.

Comment by otter on 3 Mar 2016 04:59 UTC
The reversions have been done in bc3041acd345cc9072a0caddfd149f267eab5e40.

Comment by sjoelund.se on 3 Mar 2016 16:37 UTC
I added #1950 to propose adding a save/restore interface

Modified by beutlich on 5 Apr 2016 08:17 UTC

I am going to work on this (previously heavily discussed) issue now and plan to fix it according to the items listed in https://github.com/modelica/Modelica/issues/1899#issuecomment-272680317 and also the improvements proposed by @MartinOtter, i.e, Modelica.Blocks.Tables.Internal.
There are two issues I'd like to discuss with @HansOlsson

If we move getTableTimeTmin/getTableTimeTmax out of the initial algorithm (as proposed by https://github.com/modelica/Modelica/issues/1899#issuecomment-272680356) we need to change the external object tableID from variable to parameter. Do you see any isse with this change?
If we move getNextTimeEvent to be called from the ctor of the external object (as proposed by https://github.com/modelica/Modelica/issues/1899#issuecomment-272680329) can we savely drop the when initial() case? Also we need to pass time to the ctor then.

@HansOlsson I have my patch ready now - except the second issue if getNextTimeEvent ( = ModelicaStandardTables_CombiTimeTable_nextTimeEvent) should also be called from the constructor, such that we can drop the when initial() case. OK for you?

Yeah, finally closing the most commented (open) issue.  :satisfied: Get the binaries from:

win32.zip
win64.zip
linux32.tar.xz
linux64.tar.xz


Toolbar not floating for long inline WYSYWYG fields

The editor toolbar is not accessible when your content is higher than the vertical viewport:
https://www.loom.com/share/984064ffddad4bb0a0ba323181033e9c

This was previously addressed but got lost in the shuffle. I'm working on a fix now.

This was tested and working at the time of the last fix, but unfortunately it appears to have resurfaced. Something different may be causing the problem this time, though. I'm going to troubleshoot.


Generate move assignment operator

Now Bond generates copy/move constructor and copy assignment operator. It should also generate move assignment operator.

Move assignment operator generation has been added in commit 0d911ca9f0245b814ba578deaefb441078a60a3f. For MSVC 2013, it's implemented in terms of swap, and for other compilers it's implemented use = default.


Here's a macro_rules! const-compatible implementation

macro_rules! cstr {
    ($s:expr) => {{
        use {::core::mem::ManuallyDrop, ::std::ffi::CStr};

        #[repr(C)]
        struct Concat<A, B>(A, B);
        union Transmute<From, To> {
            from: ManuallyDrop<From>,
            to: ManuallyDrop<To>,
        }

        const S: &str = $s;
        const S_LEN: usize = S.len();
        const S_BYTES: &[u8; S_LEN] = unsafe { &*(S as *const str as *const [u8; S_LEN]) };

        const CSTR_LEN: usize = {
            let mut len = 0;
            while len < S_LEN {
                if S_BYTES[len] == 0 {
                    // to error on internal nulls:
                    panic!("cstr: string contains null byte");
                    // to truncate on internal nulls:
                    break;
                }
                len += 1;
            }
            len
        };
        const CSTR_BYTES: &[u8; CSTR_LEN] = unsafe {
            &ManuallyDrop::into_inner(
                Transmute {
                    from: ManuallyDrop::new(Concat(*S_BYTES, b'\0')),
                }
                .to,
            )
        };

        unsafe { CStr::from_bytes_with_nul_unchecked(CSTR_BYTES) }
    }};
}

The notable features this utilizes are

1.57: panicking in constants
1.58: dereferencing raw pointers in constants
1.59: CStr::from_bytes_with_nul_unchecked
(1.61: pointer arithmetic in constants, for the $s:literal branch to accept byte string literals)

The emitted code works perfectly in 1.59 or later.
Compared to the current procedural macro implementation, this declarative macro implementation:

Only accepts string literals and not byte literals. This can be fixed with a macro branch that takes $s:literal, but it's not exactly pretty... I've included it in the below elaborated implementation, though. (Unfortunately, there's no such duck-typing hope for non-literal items.)
Accepts other constant &str items/expressions!
Suffers from mixed-site hygiene... so the proper implementation (below) is a bit different to avoid name conflicts (which result in a very ugly const cycle error) and improve errors (by using a lot of helpers that ensure the only error is "expected &str" and not trying to duck-type the argument).

Elaborated implementation
[playground]
Honestly, I'd personally probably just drop the $s:literal arm.
#[doc(hidden)]
pub use std::ffi::CStr;

#[repr(C)]
#[doc(hidden)]
pub struct Concat<A, B>(A, B);

#[doc(hidden)]
pub const unsafe fn transmute_prefix<From, To>(from: From) -> To {
    use std::mem::ManuallyDrop;

    #[repr(C)]
    union Transmute<From, To> {
        from: ManuallyDrop<From>,
        to: ManuallyDrop<To>,
    }

    ManuallyDrop::into_inner(
        Transmute {
            from: ManuallyDrop::new(from),
        }
        .to,
    )
}

#[doc(hidden)]
#[allow(unreachable_code)]
pub const fn cstr_len_for(s: &str) -> usize {
    let mut len = 0;
    while len < s.len() {
        if s.as_bytes()[len] == 0 {
            // to error on internal nulls:
            panic!("cstr: string contains null byte");
            // to truncate on internal nulls:
            break;
        }
        len += 1;
    }
    len
}

#[doc(hidden)]
pub const fn str_len(s: &str) -> usize {
    s.len()
}

#[doc(hidden)]
pub const unsafe fn str_as_byte_array<const N: usize>(s: &str) -> [u8; N] {
    *(s.as_ptr() as *const [u8; N])
}

#[macro_export]
macro_rules! cstr {
    // Arm for "string literals" and b"byte string literals".
    ($s:literal) => {unsafe {
        #[allow(unreachable_code)]
        const __CSTR: [u8; unsafe {
            let mut len = 0;
            while len < $s.len() {
                if *$s.as_ptr().add(len) == 0 {
                    // to error on internal nulls:
                    panic!("cstr: string contains null byte");
                    // to truncate on internal nulls:
                    break;
                }
                len += 1;
            }
            len
        }] = unsafe {
            $crate::transmute_prefix($crate::Concat(
                *($s.as_ptr() as *const [u8; $s.len()]),
                b'\0',
            ))
        };
        $crate::CStr::from_bytes_with_nul_unchecked(&__CSTR)
    }};

    // Arm for &str-typed constant items.
    ($s:expr) => {unsafe {
        const __CSTR: [u8; $crate::cstr_len_for($s)] = unsafe {
            $crate::transmute_prefix($crate::Concat(
                $crate::str_as_byte_array::<{$crate::str_len($s)}>($s),
                b'\0',
            ))
        };
        $crate::CStr::from_bytes_with_nul_unchecked(&__CSTR)
    }};
}

Hello there!
Thanks for writing this up! Nevertheless, I don't think a declarative macro based implementation pulls its weight, because the procedural macro:

is easier to read,
results in only one unsafe block (compared to 3 or 7 in the respective implementations above),
and avoids the slightly awkward #[doc(hidden)] public-but-not-really items.

transmute_copy is within striking distance of being stabilized, so the Transmute type / transmute_prefix can be dropped when that happens. The main advantage of the decl macro is being able to use it for const items/expressions (e.g. a const S: &str or concat!); if you only need/want literals, then that's not even part of the benefit equation I guess and the proc macro is probably cleaner.

Yes, this was indeed primarily intended as a convenience replacement for literals only. (I myself am using this in a database access layer, where hard-coded SQL is sent to the SQLite FFI.) If a string expression involved more complex operations such as concat!(), then I wouldn't mind having to do the occasional explicit conversion, so using this macro would have much less benefit in the first place.


Stuck while switching image to video and vice versa

Describe the bug
When I am trying to switch from image mode to video mode the camera screen is stuck with the previous image instead of showing realtime camera view
To Reproduce
Steps to reproduce the behavior:

Go to '...'
Click on '....'
Scroll down to '....'
See error

Expected behavior
A clear and concise description of what you expected to happen.
Screenshots
If applicable, add screenshots to help explain your problem.
Environment (please complete the following information):

Device: [e.g. iPhone6]
OS: [e.g. iOS8.1]
Xcode Version [e.g. 9.4.1]
Swift Version [e.g. 4.1]

Installation Type

Carthage
Cocoapods
Manual

Additional context
Add any other context about the problem here.

There is 2022 year outside🎉 Old outdated issues are closing. Please search if there are already existing issues with the same problem before opening new ☮


Tests not running properly?

I typed this command :
php bin/propel test:prepare --vendor=mysql

And the output is :
Building fixtures in tests/Fixtures/bookstore                 OK
Building fixtures in tests/Fixtures/bookstore-packaged        Execution of target "insert-sql" failed for the following reason: /Users/shanielharrar/Sites/github/propel2/tools/generator/build-propel.xml:191:1: You haven't provided an sqldbmap, or the one you specified doesn't exist: /Users/shanielharrar/Sites/github/propel2/tests/Fixtures/bookstore-packaged/build/sql/sqldb.map
    [phing] /Users/shanielharrar/Sites/github/propel2/tools/generator/build-propel.xml:191:1: You haven't provided an sqldbmap, or the one you specified doesn't exist: /Users/shanielharrar/Sites/github/propel2/tests/Fixtures/bookstore-packaged/build/sql/sqldb.map
OK
Building fixtures in tests/Fixtures/namespaced                OK
Building fixtures in tests/Fixtures/reverse/mysql                 [phing] SQLSTATE[42000] [1049] Unknown database 'reverse_bookstore'
OK
Building fixtures in tests/Fixtures/schemas                       [phing] SQLSTATE[42000]: Syntax error or access violation: 1049 Unknown database 'bookstore_schemas'
OK

You have to create the two following database schemas: reverse_bookstore and bookstore_schemas.

This shows up :
Building fixtures in tests/Fixtures/bookstore                 OK
Building fixtures in tests/Fixtures/bookstore-packaged        Execution of target "insert-sql" failed for the following reason: /Users/shanielharrar/Sites/github/propel2/tools/generator/build-propel.xml:191:1: You haven't provided an sqldbmap, or the one you specified doesn't exist: /Users/shanielharrar/Sites/github/propel2/tests/Fixtures/bookstore-packaged/build/sql/sqldb.map
[phing] /Users/shanielharrar/Sites/github/propel2/tools/generator/build-propel.xml:191:1: You haven't provided an sqldbmap, or the one you specified doesn't exist: /Users/shanielharrar/Sites/github/propel2/tests/Fixtures/bookstore-packaged/build/sql/sqldb.map
OK
Building fixtures in tests/Fixtures/namespaced                OK
Building fixtures in tests/Fixtures/reverse/mysql             OK
Building fixtures in tests/Fixtures/schemas                       [phing] SQLSTATE[42000]: Syntax error or access violation: 1049 Unknown database 'contest'
OK

So, you also need a contest database :p

Great. After creating all the databases needed it worked out :) (All tests passed)
I'm sure it would be helpful to put this in the documentation.

Agreed. You should write a PR for that ;)

What's PR?

Pull Request sir! http://help.github.com/send-pull-requests/

Oh, I know what's pull request, Didn't recognized the abbr.
You mean to modify the README.md and open pull request? right?

something like that, yes!


Claiming Job Completed when GhostScript Runs out of Memory, Causing Print Failed

Environment:

Debian Wheezy
CUPS 1.5.3
Gutenprint v5.2.9

Steps to reproduce:

(Optional)Disable swap
Somehow fill the RAM of the machine until it has somewhere like 50MB free RAM left.
Print anything(e.g. test page)

Result:
It printed nothing, nor the printer received any print command. However, in /var/log/cups/access_log:
localhost - - [10/May/2016:14:37:12 +0800] "POST /printers/Canon_MG2400_series HTTP/1.1" 200 422 Print-Job successful-ok

In addition, in /var/log/cups/error_log:
D [10/May/2016:14:37:20 +0800] [Job 136] Last OS error: Cannot allocate memory
D [10/May/2016:14:37:20 +0800] [Job 136] GPL Ghostscript 9.05: Unrecoverable error, exit code 1

Expected result:

The print should not be shown as "Completed" or "Successful"
The log level of the error should not be "error" instead of "debug".

It wasted me a few hours to figure out what's wrong. :(

It is up to Ghostscript (or the wrapper around it) to report errors as errors. CUPS sees the stderr output as a debug message and logs accordingly.

True. But the exit code of Ghostscript is 1 instead of 0. I think, if possible, CUPS should report the message as an "error" message instead of "debug" message. That would make debugging this issue much easier.
Or is Ghostscript called by GutenPrint, causing the exit code of Ghostscript not accessible by CUPS?

The wrapper script may be consuming the Ghostscript exit status - please report that upstream to the Ghostscript folks.
Similarly, it is up to Ghostscript to tell CUPS its message is an error (via an ERROR: prefix), otherwise CUPS will assume it is a debug message (which has been the documented interface with CUPS since 1998...)

Ah. Got it. Thank you very much for quick reply. That's very helpful. :)

I think such issues should not be reported to Ghostscript
because Ghostscript upstream usually rejects such
reports as "invalid" for Ghostscript which is right
because Ghostscript cannot know what program
has called it and what its caller does with its output.
I suggest to report such issues to the matching upstream
project of the caller of Ghostscript (i.e. the "wrapper script")
because in the end the caller pf a program is responsible
to do "the right things" when calling the program in a
particular environment (here in the environment of how
the CUPS print job processing works).
The caller of Ghostscript could be something like foomatic-rip
(check your PPD file what "cupsFilter" entries there are).
If your PPD file /etc/cups/ppd/<queue_name>.ppd contains

cupsFilter ... foomatic-rip

the Ghostscript caller is foomatic-rip which is nowadays
provided by the cups-filters software (which is not from CUPS).
The cups-filters software is from OpenPrinting.org, see
http://www.linuxfoundation.org/collaborate/workgroups/openprinting/cups-filters
Otherwise inspect you /var/log/cups/error_log
what the caller of Ghsotscript is in your case.
For example if you /var/log/cups/error_log contains

Started filter /usr/lib/cups/filter/gstoraster
...
Ghostscript command line: /usr/bin/gs ... -sDEVICE=cups ...

then /usr/lib/cups/filter/gstoraster is the caller of Ghostscript
and this one also belongs to cups-filters.

This is the matching report at Ghostscript
http://bugs.ghostscript.com/show_bug.cgi?id=696767
Because
https://sadale.net/40/solution-to-cups-claiming-print-job-completed-despite-printing-nothing
contains "Raspberry Pi" see also the related issue
http://bugs.ghostscript.com/show_bug.cgi?id=696348
which is also about a system with too little memory.

I've found the following in the log:
Started filter /usr/lib/cups/filter/bannertopdf
Started filter /usr/lib/cups/filter/pdftopdf
Started filter /usr/lib/cups/filter/gstoraster
...
Ghostscript command line: /usr/bin/gs ... -sDEVICE=cups ...

Does that mean that /usr/lib/cups/filter/gstoraster is the caller?

Yes, /usr/lib/cups/filter/gstoraster is the caller.
See  "Bug reports" at
http://www.linuxfoundation.org/collaborate/workgroups/openprinting/cups-filters

Thanks for making it clear. I'll file a bug report to them soon!

For completeness:
This is the matching report at cups-filters:
https://bugs.linuxfoundation.org/show_bug.cgi?id=1354


Rule: enforce-event-naming-convention

Optional rule useful for SSR apps. When used with #30 may help with naming events returned from useEvent. Enforces users to append Evt postfix for event names. Perhaps this rule should be configurable with the exact postfix value (Evt being the default one).
const buttonClickedEvt = createEvent()

const Button = () => {
  const buttonClicked = useEvent(buttonClickedEvt)

  return <button onClick={buttonClicked}>Click</button>
}

I don't like special suffixes. Because in the model you will see it without any needs.
Suffixes is required only for view-layer(React) not for effector models

In 0.7.0 we have introduced mandatory-useEvent rule, so now it's easy to check that all events are bound to scope in React components.
This rule seems obsolete for me.


Spurious ImportWarnings on 3.6+

http://bugs.python.org/issue25791 introduced a new warning when an import is attempted with neither __package__ nor __spec__ defined in the current globals dict.  This warning now appears with many Cython-compiled extension modules (such as Pandas or multidict).
Diagnosing this is a bit tedious, but Andrew Svetlov's multidict package (https://github.com/aio-libs/multidict/issues/79) is a good starting point.  Apparently the warning isn't displayed when importing the Cythonized module itself but when importing another module from the Cython module's top-level.  Case-in-point:
$ python -We -c "import multidict"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/antoine/multidict/multidict/__init__.py", line 24, in <module>
    __import__('_multidict', globals=globals(), level=1)
  File "multidict/_multidict.pyx", line 1, in init multidict._multidict (multidict/_multidict.c:14817)
    import sys
ImportWarning: can't resolve package from __spec__ or __package__, falling back on __name__ and __path__

If I move around the "import sys" line in _multidict.pyx and re-compile the package, the line number in the traceback above also changes.
Cython may therefore need to define __spec__ or __package__ when executing top-level module code. The problem is where it should get it from, so perhaps this is really a CPython issue.

@asvetlov @brettcannon @ncoghlan

__package__ is just a pre-calculated result for __name__ if '__path__' in globals() else __name__.rpartition('.')[0] (hence the "falling back on ..." wording in the warning), so the simplest way to get the warning to go away would be for Cython to start setting that attribute automatically during module initialisation.
Setting __spec__ is trickier, and would probably entail switching Cython over to using multi-phase initialisation on 3.5+: https://github.com/cython/cython/issues/1715

Ok, @ncoghlan and I managed to diagnose the issue: Cython emulates Python 2-style implicit relative imports on Python 3 (see https://bugs.python.org/issue25791#msg294668). To disable the Python 3.6 warning, one has to add from __future__ import absolute_import at the top of every Cython source file.

Maybe to note, probably had to add those future imports, but in the end on two cases had to do this change on top of that (there was no future import before):
-from . import _ufuncs
+from scipy.special import _ufuncs

From a user perspective, it's best to switch on __future__.absolute_import and use explicit relative imports (or write Py3 code), because it reduces the overhead at import time. So the warning has its merits. ;-)
I hope to find some time soon to look into multi-phase module initialisation. That should resolve the underlying problem (and then some).

Well, it's a bit surprising to have to add a __future__ import for a functionality that's actually standard i the Python version you're using ;-)

@scoder but the relative import with form . import _ufuncs did not seem to work for python modules? Then again, probably I completely misunderstood :).

Once Cython code is translated to C, its semantics are fixed. They should not change based on the Python version that you build/import the module into. CPython only avoids this issue by not allowing byte code to be reused across the Python 2/3 boundary.

You could use preprocessor macros to switch between the 2.x and 3.x variants, no?

We obviously do :-)
With "semantics" I meant Python code semantics, not C code semantics. If the Cython code was written with Python 2.x semantics (and without future import), then the generated C code must emulate those semantics also in Python 3.x. Otherwise, the C code would no longer be platform independent.

(and before you ask: yes, there are cases where users still have to care about Py2/3 semantics and compatibility in Cython code themselves, but imports are not on that list.)

I'm not sure what you mean by "platform independent" here. Obviously if I'm running a Cython-compiled module under Py3 I'm expecting it to adopt Py3 semantics, no?

Cython code uses Python 2 syntax and semantics by default, but you can switch to Python 3 syntax/semantics with a compiler option. That means that Cython will translate your code with Python 2 or Python 3 semantics, and once it's translated and the C code is generated, these semantics must not change anymore, whether you import your module into Python 2 or Python 3.
Changing source code semantics based on the runtime platform after compilation would be very surprising for users. Imagine a C compiler that compiles your code in C89 mode, and then the binary code is executed with C99 semantics because there happens to be a new libc installed in the runtime system.
A side effect of that requirement is that Cython handles many of the Py2/3 differences behind the scenes for you. Take it as a feature.

Well, if you compile your module with  cython3 or cython -3 you want Py3 semantics, what you run it in should not matter, since its one thing what language you write in and another thing entirely which language you interface to it from.

I am still a bit puzzled though, am I mistaking semantics? Why is the from . import _ufunc semantics causing a warning (with the from __future__ import absolute_import and the from scipy.special import _ufunc is not? Really just curious, I am fine with just changing it I think ;).

Changing source code semantics based on the runtime platform after compilation would be very surprising for users

Why so? What's surprising me is that it doesn't.

Imagine a C compiler that compiles your code in C89 mode, and then the binary code is executed with C99 semantics because there happens to be a new libc installed in the runtime system.

Trying to reason about Python (a language without explicit compliation step) by making analogies with C feels misleading to me. Since Cython claims to be compatible with Python semantics (not C semantics), it should follow the Python model of selecting semantics based on whatever runtime is executing the code.

A side effect of that requirement is that Cython handles many of the Py2/3 differences behind the scenes for you. Take it as a feature.

Not if that displays warnings that are not expected for the given runtime...

@pitrou that seems dangerous to me as well. In Python we do this, but that is just a hack because having two code bases is harder then doing the dance of having a single that tries to do the same in two different environments.
Why should cython copy that difficulty when it can provide compatibility without forcing the user to think about both python semantics at the same time? The cython model seems much more sensible to me. With cython you can't get the problem of trying to run a script that has been written in python 2 in your python 3 env, and then get hideous incompatibilities.

Why is the from . import _ufunc semantics causing a warning

You get the warning in that case because it's Python executing the relative import, and for that, it now requires the module to look a certain way, which extension modules don't (at least not while their module init function is executing). This can be changed, and there is an infrastructure for that, but Cython does not use that infrastructure yet. The reason is not the import but the difference between Python modules and extension modules here. This only really shows in Cython modules, because manually written extension modules do not tend to have a similar level of complexity as Cython translated ones. Especially the module level code can have an arbitrary complexity, which is unusual for non-Cython modules.

In Python we do this, but that is just a hack because having two code bases is harder then doing the dance of having a single that tries to do the same in two different environments.

The thing is, people are used to write dual py2/py3 code. Those people would be familiar with such semantics and not be surprised if Cython worked the same way.
But for some strange reason, Cython goes out of its way to behave differently. That doesn't make sense to me: Cython generally goes to great efforts to try and reproduce CPython semantics (which can be very painful from C), but on this very point it decides to do something else that's completely unexpected and produces unduly warnings that are a pain to diagnose (because, first, I didn't expect Cython to do something so ridiculous; second, I had to dig in the generated C code -- not a pretty sight -- to finally understand what was happening).

Why should cython copy that difficulty when it can provide compatibility without forcing the user to think about both python semantics at the same time?

I would rather have to think about Python semantics -- something I'm used to since I do it on a daily basis, like everyone who uses Cython probably -- than about Cython idiosyncracies (which do not seem documented anywhere).

The cython model seems much more sensible to me.

No, it's not sensible, it's ridiculous. Python 3 has been designed to break compatibility in many ways. Claiming to bridge the gap using a superset (rather than a subset) of Python is doomed to failure.

Since Cython claims to be compatible with Python semantics (not C semantics), it should follow the Python model of selecting semantics based on whatever runtime is executing the code.

Python actually does the same thing. When you run code in Python 2, it uses the Python 2 parser and generates Python 2 byte code. If you run it in Python 3, it uses a Python 3 parser and generates Python 3 byte code. If you ported the Python 2 parser to Python 3, you could pass Python 2 code into the Python 2 parser, and it could generate Python 3 byte code that runs on Python 3. If the byte code wasn't incompatible, you could also import Python 2 byte code into Python 3 and execute it there. That this has not been done yet is not my fault. ;-)
It's not true that Python has no compilation step. It compiles Python source code to Python byte code. Once that's done, the original source code semantics of that code should not change anymore, regardless of the Python version that you import that byte code into.

Claiming to bridge the gap using a superset (rather than a subset) of Python is doomed to failure.

That's not what Cython does. It gives you Python 2 semantics or Python 3 semantics. But once you've translated your code, it does not change those semantics anymore.
You can happily try to take some old Python 2 code and translate it with cython -3, i.e. in Python 3 mode. That's probably going to fail. Cython is not different from Python here either.
I think you're missing the important point that the compilation step in Cython is already the point where the decision between Python 2 and Python 3 semantics is happening, in the same way that CPython takes that decision at import/parse time. Cython just makes this more visible by splitting parsing and building/importing into separate steps.

Python actually does the same thing

No, it doesn't.  If I give the same module to Python 3, it uses Python 3 semantics.

That this has not been done yet is not my fault.

It has not been done because it would be completely counter-productive. The whole point of the Python 2 -> Python 3 transition is to have a clear break in semantics.

It's not true that Python has no compilation step

Here is what I said: "a language without explicit compliation step". Emphasis added so that you don't miss it now...

I think you're missing the important point that the compilation step in Cython is already the point where the decision between Python 2 and Python 3 semantics is happening

Please.
I'm not missing it, I'm just saying it's confusing and unexpected. I'm not the only one here. There are/were several Cython-based packages suffering from the warning described here... If I hadn't debugged this, those packages probably wouldn't be fixed still, and users would still get polluted with warnings they don't know what to do about.
But if you're fine with confusing your users and letting them painfully diagnose unexpected warnings without even documenting the issue, then why not. It's your decision.

Sorry, but it doesn't feel to me like this discussion is leading anywhere. Our users tend to be very happy about the way Cython works here, because it allows them to concentrate on more important issues than differences in Python 2/3 syntax (namely, their code and functionality). It's a very pragmatic approach that solves a real-world issue. If you are unhappy about it, I'll note your input, but I do not see this change as long as Python 2 syntax is still supported by Cython.
I have already freely admitted that the missing __package__ and __spec__ fields are something that Cython can improve on, and I've even proposed to invest my own time into solving the issue. I think that's the best I can do.
Thank you also for investing your time into debugging this issue. I was previously not aware of these warnings.

@scoder - Do you have a timeline for the first python3 only Cython version?

Depends on what you are actually asking. ;-)
Since Py2.7 is still maintained in various places until at least 2025, we'll probably keep supporting Python 2 about as long as that, or maybe longer, depending on how long other tools and libraries still support it. However, Cython has its own large user code bases out there, and they might never migrate, if only because they don't have to. The usual rule is, if your module fails to work with a new version of CPython, recompile it with a recent version of Cython. So this is probably even the wrong question to ask entirely.
Whether we switch to Python 3 syntax by default along the way, I don't know, probably yes, but it doesn't matter for the purpose of this question. You can just set the language_level=3 option in Cython, or call cython3, and you get Python 3 syntax and semantics from the compiler, regardless of whether you use your module in Py2 or Py3 afterwards. And that's entirely intentional. The same applies to language_level=2, if we ever switch to Py3 syntax by default. I very much encourage Cython users to write their code in Py3 syntax, simply because it's cleaner in many ways. But it's entirely up to them and doesn't impact the portability of the generated C code.
Does that answer your question, or were you asking something else?

Support for PEP 489 / #1715 is implemented in #1794

Hello everyone. What is the status of this issue? I think I updated cython to the most recent in miniconda, but I still get this warning.
> conda list | grep cython
cython                    0.27.3           py36h6ecb376_0

> conda info
Current conda install:

               platform : osx-64
          conda version : 4.3.30
       conda is private : False
      conda-env version : 4.3.30
    conda-build version : not installed
         python version : 3.6.0.final.0
       requests version : 2.18.4
       root environment : /Users/fil/miniconda3  (writable)
    default environment : /Users/fil/miniconda3
       envs directories : /Users/fil/miniconda3/envs
                          /Users/fil/.conda/envs
          package cache : /Users/fil/miniconda3/pkgs
                          /Users/fil/.conda/pkgs
           channel URLs : https://repo.continuum.io/pkgs/main/osx-64
                          https://repo.continuum.io/pkgs/main/noarch
                          https://repo.continuum.io/pkgs/free/osx-64
                          https://repo.continuum.io/pkgs/free/noarch
                          https://repo.continuum.io/pkgs/r/osx-64
                          https://repo.continuum.io/pkgs/r/noarch
                          https://repo.continuum.io/pkgs/pro/osx-64
                          https://repo.continuum.io/pkgs/pro/noarch
            config file : None
             netrc file : None
           offline mode : False
             user-agent : conda/4.3.30 requests/2.18.4 CPython/3.6.0 Darwin/17.2.0 OSX/10.13.1
                UID:GID : 501:20

When I run my code normally, I get no warnings, but when I treat warnings as errors (either via -Werror or warnings.simplefilter("error") I get warnings (converted to exceptions) again.

@jebob That's probably because the default filter list for python excludes ImportWarnings.  'default' not to be confused with -Wdefault, which simply means 'warn once'.  The default python warnings filter list is:
# Show deprecation warnings in __main__ once
default::DeprecationWarning:__main__
# Ignore these other warnings altogether.
ignore::DeprecationWarning
ignore::PendingDeprecationWarning
ignore::ImportWarning
ignore::ResourceWarning

Unfortunately, you can't get around this warning without disabling ImportWarnings altogether (at least in my case, dealing with it while using Pandas), since the ImportWarning comes from importlib itself.


UI-REACT: Confirm transaction modal auto closing in Tonkeeper browser

Describe the bug

So, in localhost and TMA - sendTransaction works perfectly.
But, If I use my app in Tonkeeper browser - it immediately closes the confirm transaction popup, so I can't make the transaction.
Example:
https://github.com/ton-connect/sdk/assets/59208843/8655dd09-352a-416b-9a48-cd2e770f0ec9
My code:

If I add config param into sendTransaction method { modals: ['before', 'success', 'error'] }
App won't show me confirm transaction popup. Instead of this, it will show me canceled transaction popup.
Example:
https://github.com/ton-connect/sdk/assets/59208843/ce259215-68b3-4490-bffd-22a8adbdf730
My package.json:
{   "name": "XXXXXXXXXX",   "version": "0.1.0",   "private": true,   "dependencies": {     "@chakra-ui/react": "^2.8.0",     "@craco/craco": "^7.1.0",     "@emotion/react": "^11.11.1",     "@emotion/styled": "^11.11.0",     "@kloktunov/react-telegram-webapp": "^1.1.3",     "@reduxjs/toolkit": "^1.9.5",     "@telegram-auth/react": "^1.0.3",     "@testing-library/jest-dom": "^5.14.1",     "@testing-library/react": "^13.0.0",     "@testing-library/user-event": "^13.2.1",     "@tonconnect/ui-react": "^2.0.2",     "@types/jest": "^29.5.4",     "@types/node": "^16.7.13",     "@types/react": "^18.0.0",     "@types/react-dom": "^18.0.0",     "crypto-js": "^4.2.0",     "dayjs": "^1.11.10",     "ethers": "^6.13.1",     "framer-motion": "^10.16.4",     "html-react-parser": "^5.0.7",     "i18next": "^23.5.1",     "i18next-browser-languagedetector": "^7.1.0",     "i18next-http-backend": "^2.2.2",     "lodash": "^4.17.21",     "lottie-react-web": "^2.2.2",     "react": "^18.2.0",     "react-dom": "^18.2.0",     "react-google-recaptcha-v3": "^1.10.1",     "react-hook-form": "^7.47.0",     "react-i18next": "^13.2.2",     "react-intersection-observer": "^9.8.2",     "react-outside-click-handler": "^1.3.0",     "react-redux": "^8.1.2",     "react-router-dom": "^6.15.0",     "react-scripts": "5.0.1",     "typescript": "^4.4.2",     "viem": "^2.7.19",     "web-vitals": "^2.1.0"   },   "scripts": {     "start": "craco start",     "build": "craco build",     "test": "craco test",     "eject": "craco eject",     "lint": "eslint . --ext .js,.ts,.tsx",     "lint:fix": "eslint . --ext .js,.ts,.tsx --fix"   },   "eslintConfig": {     "extends": [       "react-app",       "react-app/jest"     ]   },   "devDependencies": {     "@babel/plugin-proposal-private-property-in-object": "^7.21.11",     "@types/crypto-js": "^4.2.2",     "@types/react-outside-click-handler": "^1.3.3",     "@typescript-eslint/eslint-plugin": "^6.5.0",     "@typescript-eslint/parser": "^6.5.0",     "eslint": "^8.48.0",     "eslint-config-prettier": "^9.0.0",     "eslint-config-standard-with-typescript": "^39.0.0",     "eslint-import-resolver-alias": "^1.1.2",     "eslint-plugin-import": "^2.28.1",     "eslint-plugin-n": "^16.0.2",     "eslint-plugin-prettier": "^5.0.0",     "eslint-plugin-promise": "^6.1.1",     "eslint-plugin-react": "^7.33.2",     "prettier": "^3.0.2"   },   "browserslist": {     "production": [       "chrome >= 67",       "edge >= 79",       "firefox >= 68",       "opera >= 54",       "safari >= 14"     ],     "development": [       "last 1 chrome version",       "last 1 firefox version",       "last 1 safari version"     ]   } }
My index.tsx:

Expected behavior
App opens ton-connect confirm transaction modal and then tonkeeper confirm transaction modal
Current behavior
1.ton-connect confirm transaction modal blinks and tonkeeper confirm transaction modal doesn't appear
OR
2. ton-connect canceled transaction modal opening instead of confirm transaction modal
Steps to Reproduce

put app link into Tonkeeper browser search bar
click on black send ton tx button

Environment
Iphone 11
IOS 17.5.1
Tonkeeper mobile app v4.6.3
"@tonconnect/ui-react": "^2.0.2"
Additional context
No response

UPD:

Connection is restored, wallet persist and connected - but the app throw an error

I get the same error: [TON_CONNECT_SDK_ERROR] P: App tries to send rpc request to the injected wallet while not connected. Unknown app only in Tonkeeper Browser for all sendTransaction. What does this error mean?

@unsphere  So, for me, the issue causes due to different base URL of tonconnect-manifest file. If your manifestURL in TonConnectProvider is https://dev-my-site/tonconnect-manifest.json but you view as example prod version of the app https://prod-my-site - the manifestURL should have THE SAME BASE URL with your current app. Of course You need to have tonconnect-manifest.json in a public folder.
It's strange because in all browsers except Tonkeeper browser if you have tonconnect-manifest.json file on another domain - you can use it without any errors, but for Tonkeeper browser you should use the same domain - and it will work.
I hope it was able to solve your issue.


Add Globat to Hosting section

The logo you provided is too big.
Here's a compressed version that you can use.

Thank you.  I have submitted the change.


Print out image size (bytes) changes summary after resize

Chore that needs to be done:

[ ] Add newsfragment pipenv run towncrier create [issue_number].[type].rst

Types are defined in the pyproject.toml, issue_numer either from issue tracker or the Pull request number

Codecov Report
Patch coverage: 36.84% and project coverage change: -1.30 :warning:

Comparison is base (fcd7938) 52.63% compared to head (bf3e811) 51.33%.

:exclamation: Current head bf3e811 differs from pull request most recent head 997a3e5. Consider uploading reports for the commit 997a3e5 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #28      +/-   ##
==========================================
- Coverage   52.63%   51.33%   -1.30%
==========================================
  Files           4        5       +1
  Lines         171      187      +16
==========================================
+ Hits           90       96       +6
- Misses         81       91      +10

Flag
Coverage Δ

unittests
51.33% <36.84%> (-1.30%)
:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

resizer/tools.py
16.66% <16.66%> (ø)

resizer/resize.py
37.93% <46.15%> (+1.13%)
:arrow_up:

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


How to turn pages like ViewPager ?

like https://github.com/JoanZapata/android-pdfview

You might need to use the other version as their are two versions I believe.

I'm moving at onScrollEnd based on offset, is that feasible?  and  shielding onFling method.


Optional arguments

Please describe your suggestion
Optional arguments
Please supply examples of the desired inputs and outputs
Normal usage:
User enters this:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withOptionalArgument(new IntegerArgument("optional"), 10)
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1]; // This is 10 if not existant
  })
  .register();

Basically gets converted into this:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withArgument(new IntegerArgument("optional"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1];
  })
  .register();

and
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1]; // This is 10, args.length = 2
  })
  .register();

Using annotations:
Either something like this:
@Subcommand("mycommand")
@Arg(name = "int", type = IntegerArgument.class)
@OptionalArg(name = "int", type = IntegerArgument.class, optional = "10")
public void onCommand(int arg1, int arg2) {
  //...
}

or something like this:
@Subcommand("mycommand")
@Arg(name = "int", type = IntegerArgument.class)
@Arg(name = "optional", type = IntegerArgument.class)
public void onCommand(int arg1, @Default("10") int arg2) {
  //...
}

Cons of this via annotations is that the value must be a string, primitive, enum, annotation or class, which is really restrictive.

Emmm, so is it really implemented?
I like this feature, but I can't find it in v7.0 :(

Emmm, so is it really implemented?
I like this feature, but I can't find it in v7.0 :(

@SNWCreations No, this feature is not implemented (yet?)

Emmm, that's too bad :(
If so, will it be available in v7.x? (I'm still working on 1.16.5, I need it!)

@SNWCreations
This feature, despite how useful it is, is not relatively high on priority at the moment. This is because this feature can be implemented using other methods, for example, something like this:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withOptionalArgument(new IntegerArgument("optional"), 10)
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1]; // This is 10 if not existant
  })
  .register();

Can already be implemented as this:
public void execute(CommandSender sender, int arg1, int arg2) {
  // Command logic here
}

And then you implement your CommandAPICommands as follows:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    execute(sender, arg1, 10);
  })
  .register();

new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withArgument(new IntegerArgument("optional"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1];
    execute(sender, arg1, arg2);
  })
  .register();

This feature is also surprisingly difficult to implement and ends up causing more ambiguity and issues than explicitly stating everything manually.

@SNWCreations This feature, despite how useful it is, is not relatively high on priority at the moment. This is because this feature can be implemented using other methods, for example, something like this:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withOptionalArgument(new IntegerArgument("optional"), 10)
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1]; // This is 10 if not existant
  })
  .register();

Can already be implemented as this:
public void execute(CommandSender sender, int arg1, int arg2) {
  // Command logic here
}

And then you implement your CommandAPICommands as follows:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    execute(sender, arg1, 10);
  })
  .register();

new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withArgument(new IntegerArgument("optional"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1];
    execute(sender, arg1, arg2);
  })
  .register();

Optional arguments is also surprisingly difficult to implement and ends up causing more ambiguity and issues than explicitly stating everything manually.

Can I use it in annotation framework?

Can I use it in annotation framework?

No.

This feature, despite how useful it is, is not relatively high on priority at the moment. This is because this feature can be implemented using other methods, for example, something like this:
new CommandAPICommand("blah")
.withArgument(new IntegerArgument("int"))
.withOptionalArgument(new IntegerArgument("optional"), 10)
.executes((sender, args) -> {
int arg1 = (int) args[0];
int arg2 = (int) args[1]; // This is 10 if not existant
})
.register();
Can already be implemented as this:
public void execute(CommandSender sender, int arg1, int arg2) {
// Command logic here
}
My reply: I can't understand this. :(

@SNWCreations
This code:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withOptionalArgument(new IntegerArgument("optional"), 10)
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1]; // This is 10 if not existant
  })
  .register();

Is the same as this code:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    execute(sender, arg1, 10);
  })
  .register();

new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withArgument(new IntegerArgument("optional"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1];
    execute(sender, arg1, arg2);
  })
  .register();

public void execute(CommandSender sender, int arg1, int arg2) {
  // ...
}

@SNWCreations
This code:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withOptionalArgument(new IntegerArgument("optional"), 10)
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1]; // This is 10 if not existant
  })
  .register();

Is the same as this code:
new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    execute(sender, arg1, 10);
  })
  .register();

new CommandAPICommand("blah")
  .withArgument(new IntegerArgument("int"))
  .withArgument(new IntegerArgument("optional"))
  .executes((sender, args) -> {
    int arg1 = (int) args[0];
    int arg2 = (int) args[1];
    execute(sender, arg1, arg2);
  })
  .register();

public void execute(CommandSender sender, int arg1, int arg2) {
  // ...
}

Thanks! I got it.

It would be very good to get this implemented (or at least have made significant progress on this) for a 8.5.0 release due late July or early August.

After just over 2 years, this has now been implemented and merged into the upcoming 9.0.0 release.

I'm so happy to see this. :D

Implemented in 9.0.0.


Switch base url to tls

Fix typo
Switch baseurl to https

This  needs a couple of fixes before we can merge it


Move source-build pipeline YAML to dotnet-release

Motivation
We would like to move the currently public source of the source-build release pipeline into a more suitable internal repository, such as dotnet-release. In addition to the move itself, the pipeline would benefit from having access to Arcade infrastructure and tooling such as Secrets Manager, both of which are available and already in use in dotnet-release.
Work required
We should copy the source-build pipeline YAML to the new repository + duplicate the existing AzDO pipeline until the current one can be de-commissioned. The latter task is performed with two main goals in mind:

the team has existing release responsibilities that could be undermined by the move
given the importance of the pipeline it is vital to have a reliable and tested fallback until we are certain that the move did not cause any regressions

Potential issues

Until the new pipeline is deemed ready / functioning the team would need to support two sources, which may result in increased maintenance in addition to future changes being commited only to one of the sources
New pipeline will not have all required permissions at creation. Certain access rights, such as access to certain variable groups, would need to be granted on first run. Additionally, other missing permissions may be discovered only after running the pipeline

Progress

[x] existing source-build pipeline YAML has been moved (copied) to dotnet-release

https://dev.azure.com/dnceng/internal/_git/dotnet-release/pullrequest/31801

[x] existing AzDO pipeline has been duplicated and pointed to the copied sources

https://dev.azure.com/dnceng/internal/_build?definitionId=1263

Clean-up of previous pipeline is tracked via a separate issue - https://github.com/dotnet/source-build/issues/3502

Sources copied and pipeline tested with appropriate permissions to used resources granted. Links available in the description. Clean-up issue created / linked.
Closing


refactor: remove custom implementation of babel/polyfill

What does this PR do?
This is the followup of homer0/projext#97 and almost a revert of #72.
When I added the custom polyfill for Babel I didn't realize that the idea of deprecating it was so the env preset could take care of adding the polyfills when they were necessary.
What I did on #72 was basically add both core-js and regenerator-runtime to the generated bundles, the entire packages... which resulted on huge builds.
On homer0/projext#97, I properly configured the env preset so it will only include the necessary polyfills, so there's no need to add the polyfill.js this plugin was adding; and since the core is now taking care again of the polyfills, there's no need to install core-js nor regenerator-runtime from here (and since they're not installed, this is a breaking change).
How should it be tested manually?
First, run projext analyze on your target to see how much space is taking core-js, then...

Install projext#next from Github and this branch.
Delete the projext version installed inside this plugin node_modules (since this is not installed from a "proper version", npm/yarn, may install the one required by the plugin inside its directory: node_modules/projext-plugin-webpack/node_modules/projext).

Now, run analyze again and core-js should be "smaller" than before, as now it's only including what it's actually needed.
Also, don't forget to...
npm test
# or
yarn test

Pull Request Test Coverage Report for Build 327

1 of 1 (100.0%)  changed or added relevant line in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals

Change from base Build 325:
0.0%

Covered Lines:
646

Relevant Lines:
646

💛  - Coveralls


Clicking on a milestone from global/assigned issues list results in HTTP 401

Gitea version (or commit ref): 1.11.0
Git version: 2.11.0
Operating system: Debian 10
Database (use [x]):

[ ] PostgreSQL
[x] MySQL
[ ] MSSQL
[ ] SQLite

Can you reproduce the bug at https://try.gitea.io:

[ ] Yes (provide example URL)
[x] No
[ ] Not relevant

Log gist:

Feb  9 18:44:53 testhost gitea 2020/02/09 18:44:53 :0: [I] <IP_ADDRESS> - zerodb [09/Feb/2020:18:44:53 +0100] "GET /xsrv/xsrv/gitea/issues?q=&type=all&state=open&labels=&milestone=5&assignee= HTTP/1.1" 401 0 "\" \"Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Firefox/68.0"
Feb  9 18:44:53 testhost apache testhost.xlt.lan <IP_ADDRESS> - - [09/Feb/2020:18:44:53 +0100] "GET /gitea/xsrv/xsrv/gitea/issues?q=&type=all&state=open&labels=&milestone=5&assignee= HTTP/1.1" 401 - "-" "Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Firefox/68.0"
Feb  9 18:50:43 testhost gitea 2020/02/09 18:50:43 :0: [I] <IP_ADDRESS> - zerodb [09/Feb/2020:18:50:43 +0100] "GET /xsrv/xsrv/gitea/issues?q=&type=all&state=open&labels=&milestone=5&assignee= HTTP/1.1" 401 0 "\" \"Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Firefox/68.0"
Feb  9 18:50:43 testhost apache testhost.xlt.lan <IP_ADDRESS> - - [09/Feb/2020:18:50:43 +0100] "GET /gitea/xsrv/xsrv/gitea/issues?q=&type=all&state=open&labels=&milestone=5&assignee= HTTP/1.1" 401 - "-" "Mozilla/5.0 (Windows NT 10.0; rv:68.0) Gecko/20100101 Firefox/68.0"

Description
My gitea instance is available under gitea/ in my virtualhost (https://testhost.xlt.lan/gitea). All links to milestones are working fine, except when clicking on a milestone from the global issues list (eg. https://testhost.xlt.lan/gitea/issues?type=assigned&repo=0&sort=&state=open). As you can see in the log above, this link incorrectly inserts gitea/ between the repository part of the URL and the issues/ URL component.
For me clicking on this link results in a HTTP Basic auth prompt and a 401 error.

@nodiscc is this still the case?

I think it is. Didn't want to reopen until I have some time to test again (this was a temporary Gitea instance, will probably redeploy this week), I'll keep you informed.

Sorry for being late, this is still on my radar, I have an unrelated issue preventing me from installing GItea, I will come back with updates ASAP.

Appears these are both broken links really. They should just redirect to the milestones page.


Rename project to qhub-onprem to more clearly state goals

Project has been renamed

Project has been renamed


Fixed broken links in README.md

The commit 2646409 moved the documentation files without updating the links in the main README.md

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at<EMAIL_ADDRESS>Thanks!

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Oh, thanks.