3 ms·

Sorry, but this is really not suitable for much. There is so much wrong with this implementation that I’m not sure where to start. Every call to your template w

by orf 5y ago

Sorry, but this is really not suitable for much. There is so much wrong with this implementation that I’m not sure where to start. Every call to your template writes to a single file called “index.html”[1] which is then copied to another file when “written”[2]? You also print exceptions rather than allowing people to handle them as they wish. The idea is nice, but try and remove all file system calls and do this entirely in-memory. This may mean completely changing the API. 1. https://github.com/BrainStormYourWayIn/sierra/blob/main/src/sierra/table.py https://github.com/BrainStormYourWayIn/sierra/blob/main/src/... 2. https://github.com/BrainStormYourWayIn/sierra/blob/main/src/sierra/directory_structure.py https://github.com/BrainStormYourWayIn/sierra/blob/main/src/...

redfox2 5y ago
Yeah, you're absolutely right, I wasn't able to really think of another way to implement it. [2] was meant to be implemented with Flask, which is optional, so the files can directly be written to templates/ and static/. We're actually changing this. Can you give a few suggestions on how it must be implemented? We want to work on this
- orf 5y ago
  Unlike Ruby Python doesn't have great facilities for writing making implicit APIs like this: with Table(): header(...) row(...) You can do something like this but I'd recommend against it as it involves a lot of magic and has trade-offs which make it an uncommon choice. Instead you'd do something like: with Table() as table: table.header(...) table.row(...) This follows through to your template API as well. So you'd want to construct an object that represents your template, and have methods on that to add a table. For example: page = sierra.Page(...) with page.div(...), page.add_table() as table: table.header(...) table.row() print(page) # __str__() should return the raw HTML response This simplifies things, and you can use an in-memory StringIO object rather than a file. You can then return this string to flask as a text/html response and avoid Jinja2 entirely. Also don't use CamelCase, please! It looks very unpythonic. However you're really trying to create an API for producing a tree of nodes (some of which might logically map to individual tags, some of which might not) which are then serialized to HTML. I'd recommend looking at how you can use BeautifulSoup for a lot of this. BS4 contains a lot of classes and functions for creating trees of HTML-like nodes, so you could use this rather than a StringIO object and directly writing "<div>" and having to support all of the attributes as method arguments. You could just use "soup.new_tag(element, *kwargs)" and only do something "special" when handling pandas dataframes or other stuff (like ol/ul tags). Check out the BS4 documentation and methods for tree manipulation here: https://www.crummy.com/software/BeautifulSoup/bs4/doc/ https://www.crummy.com/software/BeautifulSoup/bs4/doc/
  - redfox2 5y ago
    'This simplifies things, and you can use an in-memory StringIO object rather than a file. You can then return this string to flask as a text/html response and avoid Jinja2 entirely.' I didn't think of this until you mentioned this, so might do that. But I don't get the 'avoid Jinja2 entirely' part. Jinja2 is avoided with the current syntax already (which is definitely changing). About BeautifulSoup, the autoPrettify() function is actually from bs4, it is a dependency. Yeah I know, camelCase IS unpythonic Thanks again
    - orf 5y ago
      When you do “render_template()” Flask uses jinja2 - it will read it, pass it to jinja2 and render the response. This is what I meant by avoiding jinja2 entirely. Good luck!
      - redfox2 5y ago
        Oh render_template(), yeah. Thanks
      - redfox2 5y ago
        But the issue when I avoid render_template() is this: I can't add the CSS file along with the HTML one, unless I create a <style> tag at the end of the file that has all of the CSS content. Can you comment on that?
redfox2 5y ago
Also, mighty thanks for actually taking the time out to check out the codebase. This is my first proper project, and I really appreciate this
- orf 5y ago
  Its no problem! I'm sorry if it came across as harsh

by orf 5y ago

redfox2 5y ago

Yeah, you're absolutely right, I wasn't able to really think of another way to implement it. [2] was meant to be implemented with Flask, which is optional, so the files can directly be written to templates/ and static/. We're actually changing this. Can you give a few suggestions on how it must be implemented? We want to work on this

orf 5y ago
Unlike Ruby Python doesn't have great facilities for writing making implicit APIs like this: with Table(): header(...) row(...) You can do something like this but I'd recommend against it as it involves a lot of magic and has trade-offs which make it an uncommon choice. Instead you'd do something like: with Table() as table: table.header(...) table.row(...) This follows through to your template API as well. So you'd want to construct an object that represents your template, and have methods on that to add a table. For example: page = sierra.Page(...) with page.div(...), page.add_table() as table: table.header(...) table.row() print(page) # __str__() should return the raw HTML response This simplifies things, and you can use an in-memory StringIO object rather than a file. You can then return this string to flask as a text/html response and avoid Jinja2 entirely. Also don't use CamelCase, please! It looks very unpythonic. However you're really trying to create an API for producing a tree of nodes (some of which might logically map to individual tags, some of which might not) which are then serialized to HTML. I'd recommend looking at how you can use BeautifulSoup for a lot of this. BS4 contains a lot of classes and functions for creating trees of HTML-like nodes, so you could use this rather than a StringIO object and directly writing "<div>" and having to support all of the attributes as method arguments. You could just use "soup.new_tag(element, *kwargs)" and only do something "special" when handling pandas dataframes or other stuff (like ol/ul tags). Check out the BS4 documentation and methods for tree manipulation here: https://www.crummy.com/software/BeautifulSoup/bs4/doc/ https://www.crummy.com/software/BeautifulSoup/bs4/doc/
- redfox2 5y ago
  'This simplifies things, and you can use an in-memory StringIO object rather than a file. You can then return this string to flask as a text/html response and avoid Jinja2 entirely.' I didn't think of this until you mentioned this, so might do that. But I don't get the 'avoid Jinja2 entirely' part. Jinja2 is avoided with the current syntax already (which is definitely changing). About BeautifulSoup, the autoPrettify() function is actually from bs4, it is a dependency. Yeah I know, camelCase IS unpythonic Thanks again
  - orf 5y ago
    When you do “render_template()” Flask uses jinja2 - it will read it, pass it to jinja2 and render the response. This is what I meant by avoiding jinja2 entirely. Good luck!
    - redfox2 5y ago
      Oh render_template(), yeah. Thanks
    - redfox2 5y ago
      But the issue when I avoid render_template() is this: I can't add the CSS file along with the HTML one, unless I create a <style> tag at the end of the file that has all of the CSS content. Can you comment on that?

redfox2 5y ago

Also, mighty thanks for actually taking the time out to check out the codebase. This is my first proper project, and I really appreciate this

orf 5y ago
Its no problem! I'm sorry if it came across as harsh