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

Source file organization / hierarchy #77

Closed
hildogjr opened this issue Aug 27, 2017 · 21 comments
Closed

Source file organization / hierarchy #77

hildogjr opened this issue Aug 27, 2017 · 21 comments
Assignees
Labels
feature-request New features resquested.

Comments

@hildogjr
Copy link
Owner

hildogjr commented Aug 27, 2017

Hi @xesscorp / @diorcety and the other of the group. Congratulations by the code organization and comments, easy to understand.

I have one propose of file (modules) organization / hierarchy that will make more easy to add other distributor (issue #76 ) and software with XML BoM (as Eagle ...).

I think in the main code directory have to be two folders distributors (with local.py, farnel.py, rs.py, ...) and softwares ( with altium.py and kicad.py, yes, moving the get_part_groupsand other function related with KiCad's xml BoM inside).

In the module importdirective we may use:

import distributors
import softwares

To use was a generic name preceded with the module name:

kicad.get_part_groups(...)
altium.get_part_groups(...)

rs.get_price_tiers(...)
farnel.get_price_tiers(...)

See that this create more easy standard to create new module of sites and software and to people here contribute. And possible to create a external flag when kicost is called `kicost -i file.xml --distributors farnell rs" allowing create a personalized spread sheet with only the distributor that the user want.

I hope you understand. I did not do this in my pull request because is deep modification in the code structure (and all files to change the functions name) and I trying to solve the more important issues first, and this would make my code not merge with yours.

@glcerone
Copy link
Contributor

glcerone commented Sep 2, 2017

Hi @hildogjr,
I agree with your intent to modify the kicost organization in order to make it more easy to add new cad support or distributors. It would be great if the software re-organization will be made using a object-oriented approach (i.e. using python classes). For example, it might be useful to create a base-class "Distributor" with common methods to all the distributors and another class named "EdaTool" (Electronic Design Automation) that contains the common methods for all the Cad softwares. In this way, adding the support for new distributors (e.g. TME) or softwares (e.g. Eagle) will consists in:

  1. To inherit the base-class (e.g. Distributor);
  2. Eventually overload some common methods (e.g. get_part_xx);
  3. Implement cad-specific or distributor-specific methods;

I think that this approach will speed-up very much the process to add new distributors/softwares and results in an overall improvement of the code readability and understanding.

Thanks in advance,
glcerone

@hildogjr
Copy link
Owner Author

hildogjr commented Sep 2, 2017

@glcerone , I like edaTool, "using the name now". I was using a provisional name and didn't remember of EDA abbreviation.

I am not expert in python class and overloading, I know the general methodology for class, hierarchy and so on.

I am working in this issue and you will live the most organized as possible to the community improve it.

@xesscorp
Copy link
Collaborator

xesscorp commented Sep 4, 2017

I pushed a new branch called pickle_module to the repo. It reorganizes the distributors into a subdirectory. Each distributor is a module in a separate subdirectory. Each distributor module has to implement four subroutines: get_price_tiers, get_part_num, get_qty_avail and get_part_html_tree.

The current problems with this are:

  1. It doesn't work with multiprocessing because of some problem with pickling a module.
  2. The local distributor isn't working.
  3. It doesn't yet scan the distributors directory to automatically import any distributor modules that are present.

You can take a look. It's one way of reorganizing. It gives you an object-oriented approach because each distributor module is an object with encapsulated data and a consistent interface.

@hildogjr hildogjr closed this as completed Sep 4, 2017
@hildogjr hildogjr reopened this Sep 4, 2017
@xesscorp
Copy link
Collaborator

xesscorp commented Sep 5, 2017

The pickle_module branch is now working correctly. You can take a look at it and see how it compares to your ideas. You can add new distributors by adding a new subdirectory to the distributors directory and KiCost will access it automatically.

@hildogjr
Copy link
Owner Author

hildogjr commented Sep 5, 2017

I will test soon this.

It did some improvements in the user experience, like warning (by coloring the cells) if the user to buy enough parts to the board quantity or not in stock of all distributor. I request a merge in the master branch, but before is good to try in different Excel versions (usually I just have LibreOffice avaliable).

@hildogjr
Copy link
Owner Author

hildogjr commented Sep 5, 2017

I tried the `pickle_module` on python3.5 and got this error message:

     File "/home/h/.local/lib/python3.5/site-packages/kicost/kicost.py", line 109, in <module>

     distributors = distributor_imports.distributors

     AttributeError: module 'kicost.distributors' has no attribute 'distributors'

@xesscorp
Copy link
Collaborator

xesscorp commented Sep 6, 2017

Yes, I'm getting the same error. For some reason, the setup.py is not installing the distributors directory. I'm working on it.

@hildogjr
Copy link
Owner Author

hildogjr commented Sep 6, 2017

Is it necessary that the setup.py install the distributor folder. Will this folder be visible as a general module in python? (This appear a little strange, in a topological point of view).
Maybe, just add some __init.py__ file in the distributor folder to do this importer job.

A like "macro" as proposed in the answer 265 in https://stackoverflow.com/questions/1057431/loading-all-modules-in-a-folder-in-python.

(To test this branch I just copied the need kicost folder to the python PATH folder, as I do every time that I want to test a local new branch).

@xesscorp
Copy link
Collaborator

xesscorp commented Sep 6, 2017

The distributor folder should be installed by setup.py as a submodule inside the kicost folder. (This is similar to what used to be done for the rs, digikey, mouser... folders.) The distributors folder and its sub-folders all have the init.py files. The installation appears correct when I install from my local directory, but not when I install from Github.

@xesscorp
Copy link
Collaborator

xesscorp commented Sep 6, 2017

I neglected to add the init.py file to the distributors directory. It should work now.

@glcerone
Copy link
Contributor

glcerone commented Sep 6, 2017

Hi,
I didn't tested the pickle_module yet but looking at the code organization, it seem a good step forward the objective to make the code more compact and re-usable (i.e. adding new distributors or EDA tools easier). I think that a further step will be the creation of standard Distributor and EDATools classes in order to not repeat several times code between the distributors. For example, the get_price_tiers() method is almost identical between the distributors except for the tags to find in the html tree. I think that a good way to avoid the repetition of code is to use an object oriented approach, inheriting and/or overloading these methods. I will work on it as soon as possible.

@xesscorp
Copy link
Collaborator

xesscorp commented Sep 6, 2017

Actually, if you compare all the corresponding functions in the digikey.py and farnell.py (for example), they are significantly different. You may be able to forge some common functions that can be customized with a few tags, but I have doubts. And I'm not sure if the effort would deliver any noticeable value to the user.

Of more value to the users (I think) would be things like:

  1. More distributor modules.
  2. Internationalization so users can work with their native currency.
  3. Webscraping from the correct distributor website based on a user's nationality.

I don't have any experience at items 2 & 3 so I don't know the level of effort required to do them.

@hildogjr
Copy link
Owner Author

hildogjr commented Sep 6, 2017

Worked fine now, good job, just one correction that, in may opinion, is missing. This code:

        'digikey': workbook.add_format({
            'font_size': 14,
            'font_color': 'white',
            'bold': True,
            'align': 'center',
            'valign': 'vcenter',
        .
        .
        .
            workbook.add_format({
                'font_size': 14,
                'font_color': 'black',
                'bold': True,
                'align': 'center',
                'valign': 'vcenter',
                'bg_color': '#c0c0c0'  # Lighter grey.
            }),
        ],

in the main kicost.py file have information about the color of the distributor logo (and bold, align, ... that is common).

I think this have to go out of the main file and, in same way, be imported (bg_colorand font_color) of the each distributor module. (If don't, the module are not really easy integrate, jut putting the new folder in distributors).

And the definitions HTML_RESPONSE_RETRIES, WEB_SCRAPE_EXCEPTIONS, import logger and so on may be putted in the __init.py__ of `distributors because may be understood as general definition.

About (2) and (3), would be new files in the distributors folder to deal if this issue. But let finish this and, after work the next issues. :-)

What you think about leave the altium folder as a edaTools module and altium submodule?

@xesscorp
Copy link
Collaborator

xesscorp commented Sep 7, 2017

IMO, all that format information ought to be moved into each distributor and they can install it into the distributors data structure when the distributor module is imported. Then the function in KiCost can loop through the distributors data and apply the formatting to the spreadsheet.

Yes, moving Altium to an eda_tools directory sounds like a good idea. (Python naming conventions are that modules and packages have short, lower-case names with underscores.) I guess if we do that with Altium, we will eventually have to move KiCad-specific functions to a similar directory in eda_tools, but not now.

@xesscorp
Copy link
Collaborator

xesscorp commented Sep 7, 2017

The pickle_module branch seems to be working as well as the master branch. I'll merge it into the master branch if that's OK with you. Then I'll move the format information into the distributor modules.

@hildogjr
Copy link
Owner Author

hildogjr commented Sep 7, 2017

@xesscorp , I agree (I tested too).
I am already using this pickle_module to deal with the other issues.

@xesscorp
Copy link
Collaborator

xesscorp commented Sep 7, 2017

I merged pickle_module into master and pushed it to Github. pickle_module is still there, but I'll remove it, eventually.

@hildogjr
Copy link
Owner Author

hildogjr commented Sep 8, 2017

Nice, @xesscorp.
But the HTML_RESPONSE_RETRIES (and WEB_SCRAPE_EXCEPTIONS) and other global options do not make effect in each module. I know this because of my poor connection here, I need to change this configuration in each module.
Maybe is missing some import of this variable in the distributor modules.
Because this, I checked the SEPRTR option, and appear be not used in the distributors modules despise its declaration.

@xesscorp
Copy link
Collaborator

xesscorp commented Sep 8, 2017

You're right. I've now moved the web-scraping imports and definitions into the distributor modules. SEPRTR and some other things have been removed from the distributor modules.

@hildogjr
Copy link
Owner Author

I could add HTML_RESPONSE_RETRIES option in the kicost call as kicost --retry [n].

@hildogjr hildogjr added the feature-request New features resquested. label Dec 28, 2017
@hildogjr hildogjr reopened this Mar 3, 2018
@hildogjr
Copy link
Owner Author

hildogjr commented Mar 3, 2018

New step in the hierarchy organization @diorcety, @xesscorp (to merge with the GUI). Now:

  1. __main__.py and GUI deal with the input param format (call and configurations);
  2. kicost.py deal with general program flux;
  3. New layer (modules: distributors and EDA), deal with generic requirements of each class;
  4. "sub-modules" (farnell, digikey, ..., csv, kicad, ...) deal with specific files and scrapes.

The 1 was modified moving the distributors_dict include and remove from kicost.py to __main__.py. And now GUI call kicost() (the messages still on terminal, but will be fixed).

hildogjr added a commit that referenced this issue Mar 3, 2018
@hildogjr hildogjr closed this as completed Mar 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New features resquested.
Projects
None yet
Development

No branches or pull requests

3 participants