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

feat: Allow user-specified local dir to be served by Tornado #1435

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

wang-boyu
Copy link
Member

This is an alternative implementation of #1428 and #1369, and helps implement a feature in Mesa-Geo: projectmesa/mesa-geo#76.

Some related discussions: #1364 (reply in thread) and #1428 (comment).

@@ -147,6 +148,7 @@ class VisualizationElement:
local_includes = []
js_code = ""
render_args = {}
local_dir = ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably need a comment explaining what this default behavior is. Is it relative to the directory of user launching the Python code or something else? What if local_dir is precomputed on the fly to be the __file__ of the class definition?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! Based on Tornado's source code: [1] and [2], if local_dir is absolute path such as os.path.dirname(__file__) + "/../templates", then this path will be used.

If it's relative path such as the default "" or ../src/templates, I guess the current working directory is used: see os.path.abspath.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated docstring.

@codecov
Copy link

codecov bot commented Oct 3, 2022

Codecov Report

Base: 90.85% // Head: 90.89% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (ed00e25) compared to base (4ce9021).
Patch coverage: 81.81% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1435      +/-   ##
==========================================
+ Coverage   90.85%   90.89%   +0.03%     
==========================================
  Files          15       15              
  Lines        1291     1296       +5     
  Branches      216      217       +1     
==========================================
+ Hits         1173     1178       +5     
  Misses         83       83              
  Partials       35       35              
Impacted Files Coverage Δ
mesa/visualization/ModularVisualization.py 78.75% <81.81%> (+0.68%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rht
Copy link
Contributor

rht commented Oct 4, 2022

LGTM

@rht rht merged commit 3def79d into projectmesa:main Oct 4, 2022
@rht
Copy link
Contributor

rht commented Oct 4, 2022

We need this feature in Mesa-Geo. And we have had several features merged since 1.0.0. I think we should release 1.1.0 soon? @jackiekazil @tpike3?

@jackiekazil
Copy link
Member

@rht sounds like a plan. I don't mile doing a deploy, but could use help this week putting it together. Can you start up a ticket for the release & help tag anything with the Stafford milestone https://github.com/projectmesa/mesa/milestone/31 that should be included in the history file?

@rht
Copy link
Contributor

rht commented Oct 4, 2022

IMO, the highlighted changes:

@rht
Copy link
Contributor

rht commented Oct 4, 2022

And #1413, which is a huge feature. I somehow missed this one when going through the commits.

@rht rht mentioned this pull request Oct 4, 2022
@rht
Copy link
Contributor

rht commented Oct 4, 2022

I have made #1436, but is there a way to automatically add the issues that have been closed since 1.0.0 to the history file? Adding them to the milestone seems like duplication of work.

@rht
Copy link
Contributor

rht commented Oct 4, 2022

I'm able to get all the newly closed issues with this search query: https://github.com/projectmesa/mesa/issues?q=is%3Aissue+is%3Aclosed+updated%3A%3E%3D2022-07-07. But these are clearly not representative of the commits that were merged without closing a ticket.

@wang-boyu wang-boyu deleted the modularserver_ext branch October 4, 2022 14:18
@rht rht added this to the v1.1.0 Safford Release milestone Oct 5, 2022
@rht
Copy link
Contributor

rht commented Oct 5, 2022

I used this search query is:closed updated:>=2022-07-07 and had put all the recently closed issues and PRs to the milestone.

@edgeofchaos42
Copy link

Yes 100% we need a release, particularly with the tutorial I need to eliminate the requirement for self.running=True.

Thanks @rht for getting the notes together!!!

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.

4 participants