Skip to content

Issue 45 send back diagram image#51

Merged
CopyDemon merged 74 commits intoIDAES:mainfrom
CopyDemon:issue-45-send-back-diagram-image
Jan 29, 2025
Merged

Issue 45 send back diagram image#51
CopyDemon merged 74 commits intoIDAES:mainfrom
CopyDemon:issue-45-send-back-diagram-image

Conversation

@CopyDemon
Copy link
Contributor

Fixes
Related to issue
Project board link

Proposed changes:
User can get and view diagram screenshot in Jupyter notebook

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

@CopyDemon CopyDemon marked this pull request as draft August 13, 2024 16:44
@dangunter
Copy link
Member

@lbianchi-lbl suggested a simpler way to approach all this, which I think makes sense. Use standard HTML5 browser APIs to convert the SVG element to a PNG in Javascript. Then the Playwright/Selenium etc. is not required

@CopyDemon
Copy link
Contributor Author

@dangunter when you have time please help me to review this.

@ksbeattie
Copy link
Member

@CopyDemon will look into adding some documentation for this.

@dangunter
Copy link
Member

@CopyDemon there are some conflicts in files idaes_ui/fv/static/index.html and requirements-dev.txt

@CopyDemon CopyDemon force-pushed the issue-45-send-back-diagram-image branch from fb318df to 0963887 Compare December 3, 2024 19:26
@dangunter
Copy link
Member

@CopyDemon do you know why the windows tests are failing? I can try this on my Windows machine but my main dev environment is linux now.

@CopyDemon
Copy link
Contributor Author

CopyDemon commented Dec 3, 2024

@CopyDemon do you know why the windows tests are failing? I can try this on my Windows machine but my main dev environment is linux now.
@dangunter
I have not checked that yet; I just resolved the conflict. I see that pytest said:
cannot access local variable 'save_diagram_return' where it is not associated with a value

Will look it later on my VM

@lbianchi-lbl
Copy link
Contributor

lbianchi-lbl commented Dec 3, 2024

@dangunter @CopyDemon I believe that error is due to the dictionary defined in the finally: block containing a variable that's defined inside the try: block on a line that may not be reached if the error is raised before it. I guess the fact that it only happens on Windows might be because the error is only raised on Windows.

@dangunter
Copy link
Member

@CopyDemon I added a higher-level function that encapsulates the 2 steps of visualize and save in what I think is an easier interface, I called it export_flowsheet_diagram and put it at the end of "fsvis.py"

@dangunter
Copy link
Member

dangunter commented Dec 9, 2024

@CopyDemon the tests are failing because of a bug in the way you are using try/except/finally. Remember that the finally clause is always executed, regardless of what happens in the except clause (including a return statement).

This toy example (from an ipython shell) shows the problem:

In [4]: def foo(x):
   ...:     try:
   ...:         v = int(x)
   ...:     except:
   ...:         print("failed!")
   ...:         return
   ...:     finally:
   ...:         print("finally")
   ...:         print(v)   # exception!
   ...: 

In [5]: foo("1.3")
failed!
finally
---------------------------------------------------------------------------
UnboundLocalError                         Traceback (most recent call last)
Cell In[5], line 1
----> 1 foo("1.3")

Cell In[4], line 9, in foo(x)
      7 finally:
      8     print("finally")
----> 9     print(v)

UnboundLocalError: cannot access local variable 'v' where it is not associated with a value

@dangunter dangunter self-requested a review January 27, 2025 22:54
Copy link
Member

@dangunter dangunter left a comment

Choose a reason for hiding this comment

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

Looks good


print(f"SVG saved as {flowsheet_name}_svg.svg")

current_path = os.getcwd()
Copy link
Member

Choose a reason for hiding this comment

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

what is this doing??

@CopyDemon
Copy link
Contributor Author

@dangunter All the comments are on outdated code.

@CopyDemon CopyDemon merged commit 042f565 into IDAES:main Jan 29, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority:Normal Normal Priority Issue or PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ability to send back diagram image to server

4 participants