Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New CatalogFunctions and YourDataYourRegions #4861

Closed
3 tasks done
nf-s opened this issue Oct 12, 2020 · 9 comments
Closed
3 tasks done

New CatalogFunctions and YourDataYourRegions #4861

nf-s opened this issue Oct 12, 2020 · 9 comments

Comments

@nf-s
Copy link
Contributor

nf-s commented Oct 12, 2020

Next Catalog Function

Intro

Catalog functions have function parameters, which are rendered as a form. The user then creates catalog function jobs with the form.

Catalog function jobs take those parameters, perform some unit of work (usually on a server), download results, and show results as catalog items - all job state is handled by catalog function job (eg starting, polling, error handling...)

Why

  • To make share work
    • Save catalog function state (specifically FunctionParameter state) - so values entered by user in form are saved
    • Save catalog function job state - so jobs can be shared and recovered (regardless of jobStatus - inactive, running, finished...)
  • Create simple Mixin for CatalogFunction and CatalogFunctionJob to make creating catalog functions easier
  • Typescript everything
  • Remove WPS specifics from FunctionParameters

FunctionParameters

Similar to SelectableDimensions, but is a class instead of interface, and it wraps up interaction with CatalogFunctionMixin's parameters trait - it has a setValue and clearValue method which sets catalogFunction.parameters[functionParameter.id]

A few examples of function parameters:

  • DateTimeParameter
  • StringParameter
  • PolygonParameter

CatalogFunctionMixin function parameters are defined in the implementation's get functionParameters(): FunctionParameter[]

Function parameters should be stateless other than the parameters trait (or other traits attached to the CatalogFunctionMixin, as they may be deleted and re-created as other observable/computed properties change.

For more info see CatalogFunctionMixin#functionParameters

CatalogFunctionMixin

Extends CatalogMemberMixin - adds the following:

parameters:JsonValue trait

Contains values for functionParameters.

functionParameters

@computed 
abstract get functionParameters(): FunctionParameter[];

Function parameters are rendered as ParameterEditors, their values directly map to the parameters trait. When a FunctionParameter value is modified, it will automatically update parameters trait.

When a job is created, the parameters are copied across automatically (see CatalogFunctionMixin#submitJob)

createJob()

protected abstract createJob(id: string): Promise<CatalogFunctionJobMixin>

Note: name and parameters traits are automatically copied across when submitted (see CatalogFunctionMixin#submitJob)
All user-configured job parameters should be in the parameters trait, this is the ensure that function parameter state behaves correctly, and that values can be easily copied across jobs.

Other job traits can be set in this function, as long as they aren't related to function parameters - for example the url and processIdentier trait for WPS are copied from the WPSCatalogFunction.

submitJob()

 async submitJob(): Promise<CatalogFunctionJobMixin>

Submit job:

  • create new job - see CatalogFunctionMixin#createJob
  • sets job traits (name, parameters, jobStatus, ...)
  • invokes job see CatalogFunctionJobMixin#invoke
  • adds to workbench/models (in user added data)
  • returns new job

CatalogFunctionJobMixin

Extends AutoRefreshTraits (to handle polling for job results), CatalogFunctionTraits (which only contains parameters trait) and adds the following:

logs: string[] trait

For log messages

jobStatus: "inactive" | "running" | "error" | "finished" trait

jobStatus should not be modified outside of CatalogFunctionJobMixin

_invoke()

protected abstract async _invoke(): Promise<boolean>;

Must be implemented.

Returns true for FINISHED, false for RUNNING (will then call pollForResults)

invoke()

This wraps up _invoke() and handles changes to jobStatus and refreshEnabled based on return value of _invoke()

pollForResults()

async pollForResults(): Promise<boolean>

Called every refreshInterval. Usually here you would poll a server for job status (for example WPS)

This returnstrue if job has finished, false otherwise (which will continue polling).
If the job "fails" - setOnError(message:string) is called

This behaves like AutoRefreshMixin's refreshData (implemented in CatalogFunctionJobMixin)

onJobFinish()

private async onJobFinish(addToWorkbench = this.inWorkbench)

This handles downloading job results, it can be triggered three ways:

  • _invoke() returns true (see CatalogFunctionJobMixin#invoke)
  • pollForResults() returns true (see CatalogFunctionJobMixin#refreshData)
  • on loadMetadata if jobStatus is "finished", and !downloadedResults (see CatalogFunctionJobMixin#forceLoadMetadata)

downloadResults()

    abstract async downloadResults(): Promise<
      CatalogMemberMixin.CatalogMemberMixin[] | void
    >;

Called in onJobFinish()
returns catalog members which are added to the workbench

results

results: CatalogMemberMixin.CatalogMemberMixin[]

Job result CatalogMembers - set from calling CatalogFunctionJobMixin#downloadResults (in CatalogFunctionJobMixin#onJobFinish)

Job history

Jobs are added to the My Data tab when submitted - they act like Catalog Groups - all results are members of the CatalogFunctionJob

PRs

Add show to ShortReportTraits and Tsxify ShortReport MERGED

Add refreshEnabled trait and AsyncMappableMixin to AutoRefreshMixin MERGED

Issues

  • Spinner while running
  • Ability to share with pending catalog item
  • YDYR tests

Things to discuss / Further development

  • Cancel function (if supported by catalog function job)
  • Mobxify DropDown component + Replace Select Elements with DropDown component
  • Job "edit" and "rerun" button
  • "Tour" help mode for CatalogFunction
  • ShortReportsSection UI issues... - where should logs go?
  • The way I use InfoParameters as error messages is questionable
  • Maybe use Delta UI as YDYR mode (or more generally, catalog function mode)

A CatalogFunctionJob can create multiple CatalogItems for results

  • Is there a way to visually tie a CatalogFunctionJob (or mappable GroupMixin) with its child layers?

To be done for National Map release TerriaJS/nationalmap#968

@nf-s nf-s self-assigned this Oct 12, 2020
@nf-s nf-s changed the title YourDataYourRegions New CatalogFunctions and YourDataYourRegions Oct 14, 2020
@nf-s
Copy link
Contributor Author

nf-s commented Oct 14, 2020

YourDataYourRegions

How to try out

Then:

  • Add your own CSV files
  • Try creating share link at different job stages (inactive, running, finished)
  • Try adding layers which have unsupported regions, or no data column.

PRs

Mobx dimension selector + add WMS Dimensions + TableMixin manual reigon mapping MERGED

Next: Add header option to loadText MERGED

Mobx dragdrop file fix MERGED

Mobx Legend improvements for TableMixin MERGED

  • Select useful style by default for region mapped CSVs
  • Show basic legend for region styles (styles with regions but no values)

Mobxify MapboxVectorTileCatalogItem WIP

Potential Issues

  • Download CSV (this is probably quite important)~
  • Add download button to workbench item menu
  • Add common colour scale between input layer and result layer.
  • CSV item splitter
  • List supported region mappings (i.e. when you get no available region columns in the selected layer)
  • When a YDYRCatalogItemJob is added to the workbench, it will also add all result layers ( onBecomeObserved(this, "mapItems", {...). Sometimes this doesn't work, and also, this is triggered when previewed in the data catalogue.
  • Display confidence interval in feature info
  • Ability to cancel a YDYR job
  • Get geography/side data available from API
  • YDYR button on CSV catalog Item (Or some other UI element to make it a bit more obvious)
  • User added data ID mismatch bug
  • When the user adds their own data (eg CSV), Terria gives it a unique ID based on the filename.
    • When the map is shared this data is lost (which is by design), but sometimes when the user re-adds missing data it is given a different ID - which results in error in the YDYR form.
  • Set region-mapped TableMixedIn item's rectangle
  • Set region of YDYR results manually (Remote Area is not recognised)
  • For InputLayer parameter, use layer name instead of id

Things to discuss

  • How to name all layers?
    • YDYR Catalog Function
    • Job name is YDYR ${inputLayer.name}: ${dataColumn.name}
    • Result CSV name is YDYR ${inputLayer.name}: ${dataColumn.name} Results
  • The flow of Function Parameter (fp) values
    • For example, when you swap Input Layers all other fp values will stay the same but display Invalid value ($value)
    • This was done so that if a layer is missing the user can re-add the layer and not lose fp values. A possible solution to this is to save fp values for each layer and then recall them as needed.
  • Visualise uncertainty?

To be done for National Map release TerriaJS/nationalmap#968

@nf-s
Copy link
Contributor Author

nf-s commented Oct 21, 2020

Thoughts

  • Need to get rid of show and workbench propagation from CatalogFunctionJobMixin to result catalog members
  • Only add results when job is finished
  • Only download results when job is finished
  • Make CatalogFunctionJobMixin extend GroupMixin so results show in useraddeddata
    • So add results to terria.model

All done!

@soyarsauce
Copy link
Contributor

@KeyboardSounds & @na9da to first review assumptions / context surrounding this, before looking at the PR

@soyarsauce soyarsauce assigned na9da and KeyboardSounds and unassigned nf-s Oct 26, 2020
@na9da
Copy link
Collaborator

na9da commented Oct 26, 2020

The Why looks pretty convincing to me.

While a good thing to have, is the following a common scenario?

Save catalog function job state - so jobs can be shared and recovered (regardless of jobStatus - inactive, running, finished...)

@nf-s
Copy link
Contributor Author

nf-s commented Oct 27, 2020

This was a requirement for YDYR, and would have been common for proposed work with Mahesh (that hasn't happened yet)

@KeyboardSounds
Copy link
Contributor

The architecture you've outlined here makes a lot of sense to me! I don't think I could have done better myself 😉 I particularly appreciate the attention you've paid to making this work with sharing. You should turn it into an ADR and stick it into the repo itself for posterity so that we don't lose it in the multitude of GitHub issues.

@KeyboardSounds
Copy link
Contributor

KeyboardSounds commented Oct 28, 2020

A CatalogFunctionJob can create multiple CatalogItems for results

  • Is there a way to visually tie a CatalogFunctionJob (or mappable GroupMixin) with its child layers?

This is the only big UI change I would make to v1. The way job info is currently displayed is really confusing because you don't know what each job actually produced. As a quick fix, I'd stick all of that job info into each result catalog item under a "Click to Expand" panel. Even though this results in duplication of information, I think it's still easier for users to understand. Also, put it in a monospace font, with a different colour background, to distinguish it from UI text.

I don't think the workbench should be the permanent home of job info at all, but I think that's going to take some implementing that is probably beyond the scope of v1.

@nf-s
Copy link
Contributor Author

nf-s commented Nov 3, 2020

Yep I agree with this, but I think we should get some UI/UX people onto it - which is why I have tried to not make massive changes to the UI

@nf-s
Copy link
Contributor Author

nf-s commented Nov 3, 2020

Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants