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

Add temporary files for Non-Interactive Display #1727

Merged
merged 21 commits into from
Jul 30, 2021

Conversation

chaedri
Copy link
Contributor

@chaedri chaedri commented Jul 14, 2021

  • Write display-related files to temporary files for non-interactive displays in Jupyter Notebooks. The option remains to write the display to a PNG when provided a path and filename.
  • Edit grass.jupyter demonstration notebook to include example with legend.
  • Improved Documentation and added renderer parameter to GrassRenderer

These modifications are part of an on-going Google Summer of Code project, Improved Integration of GRASS GIS and Jupyter Notebooks. You can find more information here.

You can also try out the changes suggested in this PR here:

https://mybinder.org/v2/gh/chaedri/grass/add-temp-directory?urlpath=lab%2Ftree%2Fdoc%2Fnotebooks%2Fjupyter_integration.ipynb

@chaedri chaedri changed the title Add temp directory Add temporary files for Non-Interactive Display Jul 14, 2021
@chaedri chaedri marked this pull request as draft July 14, 2021 21:27
@veroandreo
Copy link
Contributor

There's an error when creating the second instance of GrassRenderer

~/dist.x86_64-pc-linux-gnu/etc/python/grass/jupyter/display.py in __init__(self, env, width, height, filename, text_size)
     37             tmpfile = tempfile.NamedTemporaryFile(suffix=".png")
     38             self._env["GRASS_RENDER_FILE"] = tmpfile.name
---> 39         self._filename = tmpfile.name
     40         # Environment Settings
     41         self._env["GRASS_RENDER_WIDTH"] = str(width)

UnboundLocalError: local variable 'tmpfile' referenced before assignment

if the filename part is removed, then it displays the second map

@chaedri
Copy link
Contributor Author

chaedri commented Jul 14, 2021

There's an error when creating the second instance of GrassRenderer

~/dist.x86_64-pc-linux-gnu/etc/python/grass/jupyter/display.py in __init__(self, env, width, height, filename, text_size)
     37             tmpfile = tempfile.NamedTemporaryFile(suffix=".png")
     38             self._env["GRASS_RENDER_FILE"] = tmpfile.name
---> 39         self._filename = tmpfile.name
     40         # Environment Settings
     41         self._env["GRASS_RENDER_WIDTH"] = str(width)

UnboundLocalError: local variable 'tmpfile' referenced before assignment

if the filename part is removed, then it displays the second map

I think you must've clicked on the link a few minutes ago - that error shouldn't occur anymore. I fixed it in commit 3ac09fc - sorry about that!

@chaedri chaedri marked this pull request as ready for review July 14, 2021 22:05
@veroandreo
Copy link
Contributor

There's an error when creating the second instance of GrassRenderer

~/dist.x86_64-pc-linux-gnu/etc/python/grass/jupyter/display.py in __init__(self, env, width, height, filename, text_size)
     37             tmpfile = tempfile.NamedTemporaryFile(suffix=".png")
     38             self._env["GRASS_RENDER_FILE"] = tmpfile.name
---> 39         self._filename = tmpfile.name
     40         # Environment Settings
     41         self._env["GRASS_RENDER_WIDTH"] = str(width)

UnboundLocalError: local variable 'tmpfile' referenced before assignment

if the filename part is removed, then it displays the second map

I think you must've clicked on the link a few minutes ago - that error shouldn't occur anymore. I fixed it in commit 3ac09fc - sorry about that!

Yes, all good now! :)

@wenzeslaus wenzeslaus added this to In progress in Integration with Jupyter Notebooks via automation Jul 15, 2021
@wenzeslaus wenzeslaus added enhancement New feature or request gsoc Reserved for Google Summer of Code student(s) labels Jul 15, 2021
@chaedri chaedri marked this pull request as draft July 20, 2021 21:15
@chaedri chaedri marked this pull request as ready for review July 21, 2021 03:25
@chaedri chaedri marked this pull request as draft July 30, 2021 14:18
@chaedri
Copy link
Contributor Author

chaedri commented Jul 30, 2021

I switched to TemporaryDirectory after realizing that the temporary files weren't cleaning up correctly. tempfile removes temporary files after they are closed but each time a d.* GRASS module modifies the file, it opens and closes the file (thereby deleting it). Using a temporary directory avoids this because the directory is cleaned-up after "exiting the current context" which, in this case, I think means after the python kernal is restarted or shut down.

@chaedri chaedri marked this pull request as ready for review July 30, 2021 17:34
@wenzeslaus wenzeslaus merged commit 9890333 into OSGeo:master Jul 30, 2021
Integration with Jupyter Notebooks automation moved this from In progress to Done Jul 30, 2021
@wenzeslaus
Copy link
Member

@chaedri Just to clarify:

I switched to TemporaryDirectory after realizing that the temporary files weren't cleaning up correctly.

What have you actually observed? I would expect files to be left behind. I would expect them to be deleted by the named temporary file object, but then re-created by the d-commands.

'tempfile' removes temporary files after they are closed but each time a 'd.*' GRASS module modifies the file, it opens and closes the file (thereby deleting it).

The closing in the documentation refers to closing the "file descriptor" linked to the named temporary file object. The subprocess opening and closing is unrelated. However, the documentation also suggests that opening the file multiple times or from other processes may not work well or may not work on all platforms.

Using a temporary directory avoids this because the directory is cleaned-up after "exiting the current context" which, in this case, I think means after the python kernal is restarted or shut down.

When I saw your changed, I remembered we already talked about it and I think it is the way to go. I just want to make sure you didn't observed anything which would be contrary to what we understood from the documentation previously.

@chaedri
Copy link
Contributor Author

chaedri commented Aug 1, 2021

@chaedri Just to clarify:

I switched to TemporaryDirectory after realizing that the temporary files weren't cleaning up correctly.

What have you actually observed? I would expect files to be left behind. I would expect them to be deleted by the named temporary file object, but then re-created by the d-commands.

I think that is correct based on my experiments. I found the /tmp directory on my computer and could see that the files weren't being deleted, even after the kernal was shut down. Then, I tried adding the temporary file object as an attribute (self._tmpfile) so that it wouldn't close/delete. In this case, the map didn't render as expected: it only displays the last layer added to the file (for example, running d.rast for elevation then d.vect for streams will display a map of only streams but if you just run d.rast for elevation, you will get a map of elevation).

'tempfile' removes temporary files after they are closed but each time a 'd.*' GRASS module modifies the file, it opens and closes the file (thereby deleting it).

The closing in the documentation refers to closing the "file descriptor" linked to the named temporary file object. The subprocess opening and closing is unrelated. However, the documentation also suggests that opening the file multiple times or from other processes may not work well or may not work on all platforms.

Using a temporary directory avoids this because the directory is cleaned-up after "exiting the current context" which, in this case, I think means after the python kernal is restarted or shut down.

When I saw your changed, I remembered we already talked about it and I think it is the way to go. I just want to make sure you didn't observed anything which would be contrary to what we understood from the documentation previously.

I don't think I did.

@chaedri chaedri deleted the add-temp-directory branch August 17, 2021 01:46
@neteler neteler added this to the 8.0.0 milestone Dec 9, 2021
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
* Write display-related files to a temporary directory for non-interactive displays in Jupyter Notebooks.
* Named files are not suitable as we need to write to them from different processes.
* The option remains to write the display to a PNG when provided a path and filename.
* Adds renderer parameter to GrassRenderer.
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
* Write display-related files to a temporary directory for non-interactive displays in Jupyter Notebooks.
* Named files are not suitable as we need to write to them from different processes.
* The option remains to write the display to a PNG when provided a path and filename.
* Adds renderer parameter to GrassRenderer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gsoc Reserved for Google Summer of Code student(s)
Development

Successfully merging this pull request may close these issues.

None yet

4 participants