ljvmiranda921 / seagull

A Python Library for Conway's Game of Life
https://pyseagull.readthedocs.io/en/latest/index.html#
MIT License
174 stars 30 forks source link

+ lifeforms.utils.parse_cells function(s) to parse Plaintext lifeform str/files #41

Closed eugenpt closed 4 years ago

eugenpt commented 4 years ago

Hello! This is my first (~ever) pull request, so any feedback appreciated

Added parse_cells function to create Lifeform from .cells files and overall strings in Plaintext format

intended usage:

my_glider  = parse_cells('''!Name: name of the Lifeform
            ! some comment
            .O
            ..O
            OOO
            '''
        )

or simply load from local or web-hosted file:

my_glider = parse_cells('http://www.conwaylife.com/patterns/glider.cells')

Also the module lifeforms/utils contains function to parse only layout from Plaintext, which I intend to use further for potentially more readable defs of new Lifeforms (and surely more easily paste-able from .cells files of LifeWiki)

codecov[bot] commented 4 years ago

Codecov Report

Merging #41 into master will increase coverage by 0.37%. The diff coverage is 96.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
+ Coverage   97.76%   98.13%   +0.37%     
==========================================
  Files          12       13       +1     
  Lines         268      322      +54     
==========================================
+ Hits          262      316      +54     
  Misses          6        6              
Impacted Files Coverage Δ
seagull/lifeforms/wiki.py 96.29% <96.29%> (ø)
seagull/lifeforms/custom.py 100.00% <0.00%> (+8.69%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8bad223...b073bb9. Read the comment docs.

ljvmiranda921 commented 4 years ago

Hi @eugenpt ! Welcome to open-source! Let me review your PR tomorrow, it seems that codecov is throwing some coverage errors, let’s solve that together

eugenpt commented 4 years ago

Thank you for your welcome! I've tried adding tests to cover new functions in two last commits, they run fine locally, but here build system shows errors.. I should probably include urllib into requirements, or disable web-based tests (I don't know if it is even ok, seems like it's not)

eugenpt commented 4 years ago

Thank you very much for the comments!! All of them are clear and helpful and I've tried to work on each of them.

I resolved conversations for those comments I've found simple enough that I would not mess up (hopefully I did not), and I've pushed all the fixes

ljvmiranda921 commented 4 years ago

Awesome! Let me review either this weekend or the next!

eugenpt commented 4 years ago

Once again, thank you very much for the comments.

It seems I have implemented all the proposed changes, although I am not yet certain about the module docstring (have no real experience in that)

ljvmiranda921 commented 4 years ago

Approved! Thank you so much @eugenpt and congrats to your First Pull Request! Hope you enjoyed the process :smile:

Will do a squash merge on this to master. Ensure that you update your fork properly to avoid future merge conflicts! Let me know if you have any questions!

eugenpt commented 4 years ago

!!!!!! No words, just - thank you !