manusimidt / py-xbrl

Python-based parser for parsing XBRL and iXBRL files
https://py-xbrl.readthedocs.io/en/latest/
GNU General Public License v3.0
100 stars 37 forks source link

Make code and naming more standard #24

Closed Pablompg closed 3 years ago

Pablompg commented 3 years ago

I believe the way of using the library is not standard when downloading it from pypi.

Just a few non-standard issues about the library:

This is how they do it in another library which is more standard: Screenshot from 2021-05-28 12-30-52

What are your thoughts about it?

manusimidt commented 3 years ago

I would also find it much nicer if the name in PyPi exactly matches the package name. Initially I really wanted to get the pypi namespace "xbrl" for this libary. The project behind the namespace "xbrl" was never really developed and thus I asked the owner if he could transfer the namespace. But unfortunately the current owner never responded to my emails and twitter messages.

The problem with the current pypi handle "py-xbrl" is that it is a invalid name for a python package because it contains a "-". But you are right, underscores in the package name are also not really pretty.

Modules should have short, all-lowercase names. Underscores can be used in the module name if it improves readability. Python packages should also have short, all-lowercase names, although the use of underscores is discouraged. https://www.python.org/dev/peps/pep-0008/#package-and-module-names

I think I will try to contact the owner of the pypi namespace "xbrl" once more. It would be really nice if you could install the library with pip install xbrl and import it something like this:

from xbrl import parse_instance
from xbrl.instance import XbrlInstance

inst: XbrlInstance = parse_instance('./data/aapl-20211231.xml')

Thank you for your pull request, i will have a look at it shortly. I think the idea to have just one function that detects if the given file is XBRL or inline XBRL is really nice and simplifies the usage a lot. https://github.com/Pablompg/xbrl_parser/blob/c53086be6e2898da67577fd858a457f86c61580f/xbrl_parser/instance.py#L580-L585

Pablompg commented 3 years ago

The usage of - on pypi but un underscore _ in the projet name should not be a problem. That is quite standard. It is also quite standard to have a library package called py-something on pypi but to have the project name as just something in github. Then when using the library you would do it like this: from something import somethingelse.

manusimidt commented 3 years ago

@Pablompg so you would prefer if the name of the package would just be xbrl, correct? So py-xbrl in pypi and xbrl in github? I have no problem changing the name of the package and the github repository name to just xbrl instead of xbrl_parser :)

Pablompg commented 3 years ago

I think it is up to you to go with either one of the options. I think either of the following options is standard:

manusimidt commented 3 years ago

I think the first option (Renaming the package to just "xbrl" and keep the name "py-xbrl" in PyPi) is the best because then we would not have to change the name of the package in PyPi. I will change it today and then release a new Version that also includes your pull request https://github.com/manusimidt/xbrl_parser/pull/26 :)

manusimidt commented 3 years ago

@Pablompg

I will also implement the renaming of the package from xbrl_parser to xbrl into the new release (probably 1.3.0). This will introduce many breaking changes (every import statement has to be changed), thus i want to take some time for testing everything. I will probably publish the new release tonight.

Pablompg commented 3 years ago

@manusimidt Great news!

Thank you. I believe it would be better to release it with the tag 2.0.0 as this is a breaking change.

Although it is a breaking change, I believe it will be easier to work with the library from now on and everybody will benefit from it. Thank you ! :100:

manusimidt commented 3 years ago

@Pablompg yes, you are probably right. I also thought of jumping to the major version 2.x.x. Also according to https://semver.org/ it would be the right thing to do.

I just was not sure because there are so many python packages that are many years old and still on 1.x.x 😅 But we can also call the next release 2.0.0.

Pablompg commented 3 years ago

I think this issue was resolved. I can see the new py-xbrl version available at pypi and I was able to use it in production.

manusimidt commented 3 years ago

@Pablompg As you already mentioned, it would be nice if the naming would be more coherent. Thus I am thinking about renaming the github repository from xbrl_parser to py-xbrl.

What do you think of the idea? The drawback is that the old links to the repository would no longer work and we would have to update the git remote urls. But after the change, the naming would be completely coherent.