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

Release v0.16.2 #1808

Merged
merged 17 commits into from
Jun 2, 2022
Merged

Release v0.16.2 #1808

merged 17 commits into from
Jun 2, 2022

Conversation

@benjaminpkane benjaminpkane added the release Release PRs label May 31, 2022
@benjaminpkane benjaminpkane self-assigned this May 31, 2022
@brimoor
Copy link
Contributor

brimoor commented May 31, 2022

As of a624f43, when I run this code:

import fiftyone as fo
import fiftyone.zoo as foz

dataset = foz.load_zoo_dataset("quickstart-video")
clips = dataset.to_clips("frames.detections")

session = fo.launch_app(dataset)

session.view = clips

and then refresh the App by pressing ctrl + R in my browser, I get this error:

Error: invalid path support
at http://localhost:5151/assets/index.60546a6e.js:191:11861
at a (http://localhost:5151/assets/vendor.fa360434.js:51:287465)
at w (http://localhost:5151/assets/vendor.fa360434.js:51:276411)
at B (http://localhost:5151/assets/vendor.fa360434.js:51:277622)
at http://localhost:5151/assets/vendor.fa360434.js:51:278916
at http://localhost:5151/assets/vendor.fa360434.js:51:278888
at Object.F [as get] (http://localhost:5151/assets/vendor.fa360434.js:51:278910)
at getNodeLoadable (http://localhost:5151/assets/vendor.fa360434.js:51:235406)
at http://localhost:5151/assets/vendor.fa360434.js:51:277128
at u2.getLeafNode (http://localhost:5151/assets/vendor.fa360434.js:51:268153)

Also, if I then try to escape this error by pressing x in the error form, the error still persists. It also persists if I first do this:

session.view = None

and then press the x. However, I can now escape the error via ctrl + R in the browser.

I'm stating this second part because it would be ideal if pressing x would more reliably bring the App back to a functional state. One off-the-cuff idea there is:

  • If the error occurred while loading a view, reload with the view cleared (session.view = None)
  • If the error occurred while loading a dataset, reload with the dataset cleared (session.dataset = None)
  • If any other error occurred, just reload the current route

@benjaminpkane
Copy link
Contributor Author

Comments addressed.

Resetting from the error page with the X is equivalent to session.dataset = None now. Adding in more scenarios would likely get complicated.

@brimoor brimoor requested a review from a team June 2, 2022 02:24
@brimoor
Copy link
Contributor

brimoor commented Jun 2, 2022

Just verified that #1807, #1809, and #1808 (comment) are resolved 👍

@brimoor
Copy link
Contributor

brimoor commented Jun 2, 2022

Resetting from the error page with the X is equivalent to session.dataset = None now. Adding in more scenarios would likely get complicated.

Hmm not sure about always resetting to session.dataset = None... feels like this might be more annoying than helpful because it'll take clicks to get back to where you where. Likely you did a sequence of things, all of which were valid except the very last thing. I guess in that sense what would be cool is to keep a small history of pages and just pop the bad one from the stack...

But anyway, don't want to overdesign right now. The whole point of having the x is to escape to a valid state, so, I guess if it must always do one thing, then session.dataset = None is the only thing that's guaranteed to work.

@benjaminpkane benjaminpkane changed the title [WIP] Release v0.16.2 Release v0.16.2 Jun 2, 2022
@benjaminpkane benjaminpkane marked this pull request as ready for review June 2, 2022 16:48
@benjaminpkane
Copy link
Contributor Author

benjaminpkane commented Jun 2, 2022

But anyway, don't want to overdesign right now. The whole point of having the x is to escape to a valid state, so, I guess if it must always do one thing, then session.dataset = None is the only thing that's guaranteed to work.

Formalizing a history of states shouldn't be hard, but that would need to be added before we could return to the previous state.

Undrafting. I am prepping release notes now. Will publish later today.

Copy link
Contributor

@brimoor brimoor left a comment

Choose a reason for hiding this comment

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

LGTM

@brimoor brimoor merged commit 7305efc into develop Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Release PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants