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 some tests and Travis integration #2

Merged
merged 4 commits into from
Mar 23, 2018

Conversation

Peque
Copy link
Contributor

@Peque Peque commented Mar 20, 2018

These would be the results as reported by Travis:

https://travis-ci.org/Peque/mazefiles/builds/356025295

I am currently checking basic stuff about the maze file formatting and correctness. I also check that the file name only contains characters in "a-z", "A-Z", "0-9", "-" and "". I could modify that to allow only lower case in names and either "-" or "". Although that would require some name changes if we wanted the tests to pass, of course. 😊

By the way, in order to enable Travis in your mazefiles repository you need to activate that service. It is free and you can register using your GitHub account. I can do that if you want, but I would require admin access to the repository.

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 20, 2018 via email

@Peque
Copy link
Contributor Author

Peque commented Mar 20, 2018

Okay, I will change that. Some tests will fail until all names satisfy the requirements, but I guess that is fine for now.

I will also add a half-size example, as I have realized I removed the halfsize directory when deleting the .gitignore file inside it. I think I have a working basic maze image to maze text file script, which should help creating a collection of half-sized mazes (it would really be a pain to make those manually... 😂).

@Peque
Copy link
Contributor Author

Peque commented Mar 20, 2018

BTW: I will add some commits as "fixup" to previous commits, so when you think this is ready to be merged, tell me and I will first squash them.

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 20, 2018 via email

@Peque
Copy link
Contributor Author

Peque commented Mar 20, 2018

Wooops, I just changed the _ in file names too (there were only 3 occurrences, so I thought it would be better to have more homogeneous naming and just use - everywhere). I can revert that if you want.

Yeah, I will update the README file too! Good idea to document this a bit.

@Peque
Copy link
Contributor Author

Peque commented Mar 20, 2018

I do not think that is necessary. After all, the word "classic" and "halfsize" is already in the path as their respective parent directories. The Python script already checks for size consistency, according to the path (i.e.: 16x16 for files under "classic", 32x32 for files under "halfsize").

By the way, the half-size example I uploaded I found it here:

http://www.ntf.or.jp/mouse/micromouse2014/MM2014recode01.html

There are some other mazes from previous years, but it is definitely far from a complete collection. Only good news is that the script that I wrote seems to work fine with those images! (unless I missed something, the generated text file seems to be correct) 😄

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 20, 2018 via email

@Peque
Copy link
Contributor Author

Peque commented Mar 20, 2018

Updated the README file. Took the liberty of adding a "FAQ" section. Do you like it?

About the image->text script yeah, it worked surprisingly well and was surprisingly easy (the current, very basic implementation is 60 lines of code). Still need to try it with not-so-clean images to see what happens. 😂 Will share it soon too!

About marking the goal cell at (26, 5), I did not know that. Why is it important? Is not always the only section the mouse can reach that is open just like in the classic competition? (i.e.: no walls in that square/rectangle) Also, is that information provided to the contestants before the competition? As you can see, I am not very well informed about half-size competitions... 😅

Anyway, I will change that tomorrow! 😊

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 21, 2018 via email

@Peque
Copy link
Contributor Author

Peque commented Mar 21, 2018

Okay, I will add the goal then. 😊

In the README you say goal cells are marked with an asterisk. I like a "G" more. Would you agree in using the letter "G" instead?

We could also, for completeness, allow an "S" at the start cell, even if it is always at (0, 0), it could be useful for training/non-standard mazes.

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 21, 2018 via email

@Peque
Copy link
Contributor Author

Peque commented Mar 21, 2018

Pushed those changes. Tell me if there is anything else you want me to change.

If everything looks good to you I will squash all the fixups before you merge. 👍

@Peque
Copy link
Contributor Author

Peque commented Mar 21, 2018

Hmmmm... I was thinking maybe we could be less restrictive. In example:

  • We specify, as we do now, that the text maze format allows (but does not enforce) the use of "G" to mark goal cell(s).
  • We can specify in the README that halfsize must have a goal cell marked, though (I added that check to the tests). This is to comply with the international rules. I understand that "classic" and "halfsize" directories should comply with international rules.
  • But even classic mazes, they could have the goal cells (the four cells) marked with a "G", just like here: http://www.ntf.or.jp/mouse/micromouse2014/MM2014recode01.html
  • Maybe we could even force all of the mazes to have the goal cell(s) marked with a "G". All classic mazes should have the four center cells marked with a G in that case. I think this could be a good idea.
  • This allows for training mazes, for example, to have a goal cell marked anywhere. Or multiple goal cells which might even be separated. After all, training mazes do not have to comply with any international rules.

So at the end: the text maze format only specifies that goal cells are to be marked with a "G". Then, depending of the rules (if any), we apply that specification to all the maze files in the different directories.

What do you think?

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 21, 2018 via email

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 21, 2018 via email

@Peque
Copy link
Contributor Author

Peque commented Mar 21, 2018

Yeah, agree about moving non-contest mazes to training. That should be a separate pull-request though.

I pushed the changes you requested in the README file too.

Just to try to explain myself better one last time (I think maybe I did not the first time)... What I had in mind is:

  • In the list that defines the file format in the README file, we only mention that a "G" can be written to mark goal cell(s).
  • Bellow, when talking about the halfsize folder, we clearly specify that the "G" is a must and there is only one goal cell. This folder is for mazes that comply with the international halfsize rules.
  • In training mazes we could allow any configuration, not necessarily complying with classic nor halfsize international maze rules. Although that could be in a different folder... Here we could have a 50x50 maze, for example, or a 4x6 not contained in a 16x16 maze. Goal cells could be single, groups or even multiple and separated. No rules except to comply with the maze file format. 🤪

But I understand if you want to exclude that from your collection.

@Peque
Copy link
Contributor Author

Peque commented Mar 21, 2018

Actually, instead of moving non-contest mazes to training, we could prepend the word training to their names. This allows for easy filtering. While we still leave totally valid classic maze configurations in the same folder.

@Peque
Copy link
Contributor Author

Peque commented Mar 22, 2018

By the way, I just finished packaging the software for image-to-maze-file conversion:

https://github.com/Theseus/ommr

Is it fine for you if I mention it in your README? Maybe potential contributors could benefit from it making their job a bit simpler.

If you have some spare time and want to try it out I would appreciate some feedback! 😊 All the documentation (very short) is in the README file of the repository.

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 22, 2018 via email

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 22, 2018 via email

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 22, 2018 via email

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 22, 2018 via email

@Peque
Copy link
Contributor Author

Peque commented Mar 22, 2018

Yes. Include the link.

I just pushed the update in the README file. Tell me if there are any other changes you want me to do before squashing this.

If it could cope with not having the right/bottom walls cropped, it would be possible to scrape images from web pages/sites and auto-convert them.

Yeah, I already opened an issue for that. 😊

Are there any websites in particular you are thinking about scraping? I thought I could create a simple script to crop all images in http://www.ntf.or.jp/mouse/history/index.html and that should work well but then: it would only work for those images. Matching names between those images and your repo was another task that required some extra work. Would you help me with that last part if I did the first? 😉

Excuse my ignorance but - how does pip know where to find ommr?

I uploaded the package to the public PyPI repository. Once you have your setup.py file configured it is really simple. 😊

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 22, 2018 via email

@Peque
Copy link
Contributor Author

Peque commented Mar 22, 2018

About the cryptic names... I would prefer longer, explicit names like japan-2011-expert-final instead of japan2011ef. It is easier to search for "final" than "f" (would give better results). And if you want to display shorter names, removing dashes and cropping words is always easier than the reverse operation...

But I guess you do prefer shorter names, right? 😂

@micromouseonline
Copy link
Owner

micromouseonline commented Mar 23, 2018 via email

@Peque
Copy link
Contributor Author

Peque commented Mar 23, 2018

Had to search what 8.3 filenames were. 😂

Great! No hurries, of course. 👍

@Peque
Copy link
Contributor Author

Peque commented Mar 23, 2018

Squashed everything into 4 commits to hopefully make the final review a bit easier.

@micromouseonline micromouseonline merged commit bdb71f2 into micromouseonline:master Mar 23, 2018
@Peque Peque deleted the travis branch March 25, 2018 19:48
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.

2 participants