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

Load track speeds seperately #818

Merged
merged 6 commits into from
Aug 26, 2021

Conversation

nerik
Copy link
Contributor

@nerik nerik commented Aug 19, 2021

Loads track speed (or any other field in theory) separately from the main track data (latlon/timestamp)

See https://globalfishingwatch.atlassian.net/browse/MAP-469

@global-fishing-watch-bot
Copy link

Automatic is false. You need to set a label Deploy to run the deploy in dev environment

@changeset-bot
Copy link

changeset-bot bot commented Aug 19, 2021

⚠️ No Changeset found

Latest commit: c56cdc5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@nerik nerik force-pushed the fishing-map/track-speed-separately branch from f7eb2d0 to c0c0f5c Compare August 20, 2021 12:58
@nerik nerik changed the title [WIP] Load track speeds sperately Load track speeds sperately Aug 20, 2021
@nerik nerik requested a review from j8seangel August 20, 2021 13:10
@nerik nerik changed the title Load track speeds sperately Load track speeds seperately Aug 20, 2021
Copy link
Collaborator

@j8seangel j8seangel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I'd do a full round of testing once this is merge with resources-query-refacto

[
selectAllDataviewInstancesResolved,
selectThinningConfig,
selectWorkspaceStateProperty('timebarGraph'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to read the timebarGraph exists this one but I'm having this error using it:

'selectDataviewsForResourceQuerying' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.ts(7022)

seems like we have a circular dependency so we can leave it as it is for now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the reason I used selectWorkspaceStateProperty, not very clean but...

packages/dataviews-client/src/resolve-dataviews.ts Outdated Show resolved Hide resolved
if (fieldsQueryIndex > -1) {
query[fieldsQueryIndex] = {
id: 'fields',
value: 'lonlat,timestamp',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means we are not using the dataview configuration anymore until we remove speed, isn't it?
do you thing worth it to keep it there ? or do we config it directly in the app?

@j8seangel j8seangel merged commit fe9aeb4 into resources-query-refacto Aug 26, 2021
@j8seangel j8seangel deleted the fishing-map/track-speed-separately branch August 26, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants