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

Improvement: Error Messages #218

Closed
2 tasks done
2320sharon opened this issue Jan 22, 2024 · 3 comments
Closed
2 tasks done

Improvement: Error Messages #218

2320sharon opened this issue Jan 22, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@2320sharon
Copy link
Collaborator

2320sharon commented Jan 22, 2024

Tasks

  • Improve the error message when the ROIs loaded in are missing the downloaded data
  • Fix the Error Box so it scolls to the bottom
@2320sharon 2320sharon added the bug Something isn't working label Jan 22, 2024
@2320sharon 2320sharon self-assigned this Jan 22, 2024
@2320sharon
Copy link
Collaborator Author

Here is the new error message if the user loads a session where some of the ROIs loaded weren't downloaded.
I also fixed the style issue with the scroll for the error box.

Screenshot 2024-01-22 121516
Screenshot 2024-01-22 121526

@2320sharon
Copy link
Collaborator Author

The Root Issue

I realized from multiple error reports that the ROI settings were not being created if not every single ROI had been downloaded on the user's machine. This meant if someone had shared an ROI with another user who had downloaded the data for them with other ROIs, when the user who had not downloaded ALL the ROIs tried to extract shorelines with the ROI they received they would get a "no roi settings found error".

The Solution

The solution to this issue is broken into multiple parts because this bug is complicated.

  1. Even if not all the ROIs are present in the user's coastseg/data directory the ROI settings will be loaded.
  2. If not all the ROIs are downloaded when the session is loaded a warning will display showing which ROIs are missing and how to extract shorelines with the remaining ROIs.
  3. A warning will appear if the user attempts to extract shorelines for any of the ROIs that weren't downloaded. The warning tells the user to de-select the ROIs that were not downloaded. (You can see this warning in the screenshot above)

[Advanced section]

I discovered that the downloaded ROI's configs were being overwritten each time shorelines were extracted because self.save_config was being called in the extract_shorelines function. Normally, this would be fine, except this meant that if the user did not select all the ROIs to extract shorelines from then the config file would be edited to not include the ROIs that hadn't been selected. This is problematic because the config files for the downloaded ROIs were being modified to not include ROIs that had been downloaded. This meant that when the user went to load an ROI they had downloaded with other ROIs in a single session the other ROIs they downloaded would not load unless they loaded the ROIs manually.

The solution to issue described above was to not call save_config in extract shorelines and to instead add a new function called update_downloaded_configs to update the filepath for each ROI in the config.json with the user's coastseg/data location and update the config.json file. This new function,update_downloaded_configs, was added to load session so that when the user loaded in ROIs that could have been downloaded by another user the config files would be updated to hold the current user's ROI location instead of the location the ROIs were in when the original user downloaded them.

@2320sharon
Copy link
Collaborator Author

While I was working on this issue more yesterday I had another realization.
The function to load the config.json file is called within the load_session_files() function. The issue is if I raise a warning message right when I discover some roi's were not downloaded this will cause the extracted shorelines not to be rendered on the map because they are loaded on the map after the sessions are loaded. This means I'll have to raise an exception and catch the air at the end of the call to load session. In practice, this means putting the air handler around the function that is called when the load session button is pressed, which is how all the other error handlers are implemented. I just need to make sure the error message is formatted correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

1 participant