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

Split library into submodules #1

Closed
wants to merge 1 commit into from

Conversation

paramono
Copy link

Hi Curtis!

First of all, thanks for the amazing library!

I am an experienced python developer myself, but I am a complete beginner in Blender and I have been learning it as a hobby. Naturally, since I do most of my work in Python, I got interested in Blender Python APIs, to see what kind of cool things I can do with it, but I found the API quite cumbersome - when you try to create something programmatically, it gets verbose pretty quickly. You have to thoroughly study Blender documentation to figure out how to achieve even the simplest things.

So I was very excited when I saw your video on EasyBPY, because it is exactly what I was looking for - it was like a very clear set of examples of how to do most common tasks with Blender API. And of course I immediately forked it! :)

Do you accept pull requests?

I noticed that you dumped everything into one file, easybpy.py, but is it convenient to work with that kind of layout?

I decided to play around with it, and I split all your code into submodules. All of the submodules are imported in __init__.py. That means that if you want your end users to use the library, instead of copying a single file, they will have to copy the entire EasyBPY directory. However, it's much more convenient for code organization and will make it easier for everyone to work with the library.

There is one caveat with that approach, though. Since EasyBPY directory has mixed case, and the old "easybpy.py" file is in lower case, either the end users will have to be instructed to use from EasyBPY import * (mixed case) instead, or you should rename directory so it's all lowercase.

I've also made a few minor stylistic changes to the code to follow the PEP8 guidelines:
https://www.python.org/dev/peps/pep-0008/

When I see Blender community do things in Python, I notice that they generally do not follow the best Python practices. And PEP8 is a standard Python guideline that helps people write code that is stylistically consistent. Since you've been making YouTube videos on how to work with Python in Blender, I think you're in the best position to promote the best practices simply by using them in your code, so that people can learn coding in a pythonic way from your example.

I've noticed only a few minor PEP8-related things that I tried to fix with this pull request:

  1. There should be no spaces around the equal sign ("=") for function arguments' default values - https://www.python.org/dev/peps/pep-0008/#other-recommendations
  2. x is not None is preferred to x != None, same applies to booleans (x is True, x is False) are preferred - https://www.python.org/dev/peps/pep-0008/#programming-recommendations
  3. Function definitions should be surrounded by two newlines - https://www.python.org/dev/peps/pep-0008/#blank-lines
  4. There should be one space after each comma in function arguments - it seems this is not explicitly stated in PEP8, but most style checkers follow this rule, and PEP8 adheres to this style in their examples.

A few other things that I should mention:

  1. I noticed that you used #region comments, most likely to be able to collapse code in your IDE/text editor. Arguably, with the new layout, you don't really need #regions anymore, but I kept these comments for your convenience.
  2. Code related to objects and collections is dependent on each other. This means, if they're split into submodules, there's a risk of getting circular imports, since some object functions depend on collections module, and some collections functions depend on objects module. This can be solved by changing the library layout, but I didn't want to introduce traumatic changes right from the beginning. So I used a classic workaround in these modules - I inserted the imports inside the functions that require them instead of putting them at the top of the file. That's not the best solution, but it helps to avoid circular imports without reorganizing the library too much. Other modules didn't really have circular dependencies, so importing stuff in them was much more straightforward.

I ran some simple tests to make sure that the library works, but it probably needs more thorough testing.

Please let me know what you think about all this!

@curtisjamesholt
Copy link
Owner

Hi, thanks for checking out the project and sharing your thoughts - I appreciate the interest.

I won't be accepting pull requests, but I will consider all suggestions and make the changes myself just to make sure the design philosophy is upheld.

I am going to keep EasyBPY as one single file (for the regular users) to keep it simple to understand from their perspective, however at more significant version milestones we could do a split version specifically aimed at addon developers that might want to optimize their access to different categories of functions.

As for PEP8, I am aware of the standard and I agree with some of these changes. For example, changing != to 'is not' is fine - that doesn't make it harder for me to read.
However, I would like to express that I am much more of an artist than a programmer, so naturally I just don't care that much about standards - but I will try to implement some of these changes.

This project is all about user experience, the number of lines and spaces in the code will not affect this.

As for the use of regions, these are great for my workflow because it lets me fold code wherever I like which makes for effortless navigation of long files.
Since region folds are nestable, there's really no limit to size - it just feels like navigating folders, so for me that's easier to work with than multiple files. But really the primary reasons for keeping it all in one place is to make it easier for people to download, use, share and understand.

But as I say, an alternate version can certainly be done in the future for the user's personal preference and your proposal is a fantastic idea for this.

Thanks again!

For reference, this is what my workspace looks like:
image

@paramono
Copy link
Author

I see!

Thanks for a thoughtful reply, I really appreciate it!

however at more significant version milestones we could do a split version specifically aimed at addon developers that might want to optimize their access to different categories of functions.

The only thing I can add here is that maintaining another version aimed specifically for addon developers, if you envision something like this in the future, is simply increasing the amount of work for yourself. Right now the library has great potential for being usable both for Blender/Python beginners and addon developers alike, and it really looks like something that official Blender documentation lacks, because it removes all the most common pain points.

If you split it into different submodules, you make it easier for addon developers to import specific functions they need, but at the same time you're not really making it harder for beginners - they just need to copy and paste a single folder instead of a single file. That way, you kill two birds with one stone, and the same library now has multiple usage scenarios.

Another benefit of this approach is that it makes writing automated tests easier, each test file can be testing functions from a specific module. Then it makes code changes a much less stressful experience.

In any case, I hope I wasn't too annoying! It's your library and of course it's entirely up to you how to develop it.

I just really liked how it makes Blender API much less obscure and I was excited to share my thoughts about it. I am closing the pull request.

Good luck with your project!

@paramono paramono closed this Sep 12, 2020
@curtisjamesholt
Copy link
Owner

I appreciate the input!
From my perspective, off-shooting another split version for addon developers would not be much more effort. It would mostly be a matter of copy-pasting the categories from the main file - I expect this would take a maximum of a few minutes, but we'll see if and when it happens.

Quackarooni referenced this pull request in Quackarooni/EasyBPY Dec 4, 2022
Quackarooni referenced this pull request in Quackarooni/EasyBPY Dec 4, 2022
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.

None yet

2 participants