search

pyoscx / scenariogeneration

Python library to generate linked OpenDRIVE and OpenSCENARIO files
Mozilla Public License 2.0
276 stars 86 forks source link

What about a seamless PyPI experience? #1

Closed johschmitz closed 3 years ago

johschmitz commented 3 years ago

Great work! What about merging the three libraries into one and making them available through PyPI (https://pypi.org/)?

I believe your libs are designed to work together so it makes more sense to me to use them in a way similar to this

from scenariogenlib import xodr
from scenariogenlib import xosc
from scenariogenlib import helpers, xyz, something

And then the consumer can be sure that they fit together and can do:

pip install scenariogenlib

For a seamless experience.

That would be amazing for usage within other projects! Otherwise, due to the missing dependency management, people might start copying your code somewhere into there projects and your little eco system will become quite fragmented.

Keep up the good work!

mander76 commented 3 years ago

That's an excellent idea. I haven't done anything like that before though, so I have to read on how it's done.. Or if you know how to do it feel free to share :)

johschmitz commented 3 years ago

I suggest you first streamline how you want others to use your lib, for example they will run into trouble when you give them three libraries that are supposed to be working together but don't properly work together when you mix different versions of them. So versioning would be one thing you need in any case. Per version you need a changelog. In your case I would suggest to put everything into one library and then if a user only needs to write xodr, for example, he can only use the xodr module by only importing that. Your libraries are not that big so there's no real benefit in splitting them and push the integration issues further downstream. I can tell you this out of experience.. So aim for keeping everything in one single repository with a joint documentation on how to use it together. Of course if you decide you don't like that idea you can publish 3 different items on pypi every time you update.. Or you will have different version numbers for them and nobody will understand which versions will work together properly.. So a monolithic approach starts to look intereresting doesn't it?

Searching Google for "pypi how to publish" will give you a couple of results how to do it. If you made it this far it shouldn't be a big challenge for you. I just want to warn you: first do the cleanup and refactoring and move into one "mono"repo, then publish. Doing it the other way round will be hard to impossible because people already start using it and then you need to tell people to use something else. The good news is that it is pretty simple to execute because there are no functional changes:

  1. Merge everything into one library (maybe pyoscx because that is the account name? Or you want to change that too?) with sub libraries (choose one of your existing repos or a new one)
  2. Write in the Readme of the other repos that they are now deprecated and people should use the unified new version (link to the merged repo)
  3. Release a version with initial changelog on github with a Git tag.
  4. Publish to PyPI
  5. Now users can install and update your lib with a simple pip install and don't have to copy source code from your repo anymore.
mander76 commented 3 years ago

Thanks for the input, that helps a lot. I'll try start looking into this, and also discuss it with a couple of colleagues.

jakobkaths commented 3 years ago

Hi @mander76 , did you get a chance already for a decision or maybe even a rough timeline? Pypi would make things a lot easier for us and we would wait a little for further steps on our side in case you tackle this soon.

MandolinMicke commented 3 years ago

I have sadly had a very busy couple of weeks, but I have started looking into the structure, aswell as the documentation parts needed, I hope I can get some more time in the coming week to finalize it.

And after some revising, I realized some functionality might have to be rewritten and not be exactly the same as it was in pyodrx and pyoscx..

johschmitz commented 3 years ago

It is super cool that someone is actively working on this. Although many people are waiting for it, please take your time and get the software architecture right. Makes no sense to rush it and then change it again. Try getting the stuff right that you can't easily change in later versions. Always think like this: If I need to change this part later then all consumers of the lib have to make changes, okay here I need to spend a little extra time thinking about it. And then it's done when it's done.

You might wanna have a look at semantic versioning principles and what your version numbers tell the user about compatibility and breaking changes to get the right from the beginning as well: https://semver.org/ You can start with quickly iterating through 0.x.0 version numbers and make braking changes every time (bootstrapping phase) and then go to 1.x.0 numbers and make no breaking changes until moving to 2.x.0 versions. x can be larger than 9, like 0.27.0 or 1.12.0. Last digit is for hotfixes between larger x releases.

Looking forward to try out the result :-)

MandolinMicke commented 3 years ago

Yeah, my plan is to have something like that aswell, right now the main difference I'm looking into is the "helpers.py" module where the write xml, run esmini (would be nice to add more opensource players like CARLA here aswell) are coded. Those and similar would be nice to merge, hence the write_xml methods of the Scenario/OpenDrive class will be removed and placed outside.

so previously: scenario = Scenario() scenario.write_xml(filename)

will become something like: scenario = Scenario() write_xml(scenario,filename) instead.

jakobkaths commented 3 years ago

Just to clarify: I didn't want to create any rush or pressure here and to be honest I'm also not aware what it takes to get this to PyPI, I just have to try to align with our development / release cycle and was interested in a time estimate. But I'm taking that this transfer is probably still taking a while. Thanks for the comments!

mander76 commented 3 years ago

I want to do it for our internal things aswell, so that our users don't have to update 3 repos (without pip). The issue is just to find time to do it...

mander76 commented 3 years ago

Now there is a first version of the merged repo here. Please have a look, will try to fix the Pypi aswell this week.

johschmitz commented 3 years ago

Looking good! I will try it out soon. I think other people will be quite happy too. I hope it will be available through PyPI soon.

mander76 commented 3 years ago

https://pypi.org/project/scenariogeneration/

johschmitz commented 3 years ago

Nice! I have been able to install it. Will hopefully find some time to check it out tomorrow.

schottb85 commented 3 years ago

Might it be possible to provide the license text of MPL-2.0 also for the uploaded pypi package? So far only the license is referenced on pypi. For tools like pip-licenses (called with command line option --format plain-vertical --with-license-file) it is necessary to have a LICENSE.rst in the packages dist.info subfolder. I think for proper license-related handling this might be very important.

mander76 commented 3 years ago

@schottb85 Will have to check how to do that.

mander76 commented 3 years ago

@schottb85 Now it should be there.

schottb85 commented 3 years ago

That seems to work as expected. Thank's a lot. I have only one concern. You uploaded the change as new pypi versions 0.2.4, 0.2.5, 0.2.6. For us it is a lot of administrative stuff when we want to use a new package version. Would it be possible to upload the additional license file as version 0.2.3? And further, what do the versions 0.2.4 and 0.2.5 include as new features?

johschmitz commented 3 years ago

I am also a bit confused about versions. There is something like 1.0.0 but also 0.2.x? I think until API and everything is stable I would stay with 0.x.x version numbers and iterate quickly. I think everyone who wants to use it in this early unstable phase should not complain about rapid development and version number changes! Once something good and stable is reached it needs to stay API compatible (all 1.x.x versions until 2.0.0) according to semver. Also git tags and PyPI versions should be identical!

Benedikt Schott notifications@github.com schrieb am Mo., 8. März 2021, 10:25:

That seems to work as expected. Thank's a lot. I have only one concern. You uploaded the change as new pypi versions 0.2.4, 0.2.5, 0.2.6. For us it is a lot of administrative stuff when we want to use a new package version. Would it be possible to upload the additional license file as version 0.2.3? And further, what do the versions 0.2.4 and 0.2.5 include as new features?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pyoscx/scenariogeneration/issues/1#issuecomment-792789346, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF4PC2JVNN2J3DRIK44BJ3TCTM35ANCNFSM4WPZHS7A .

mander76 commented 3 years ago

@johschmitz The "1.0.0" is for coverage and is a bug (copy paste from pyoscx, will fix it tomorrow). (if it is in more than that place, please let me know)

@schottb85 Since the package is less than a week old (the merge of pyoscx, pyodrx, and the Pypi upload), there will be some rapid changes, both bug-fixes, some small features to make it work properly, and for me personal a learning curve (my first open-source project like this and my first upload to PyPI). I hope to have some stability soon, since the merging and moving around of some functionality might cause some unforeseen things.

schottb85 commented 3 years ago

Thank's for your work on this topic. It's fine for me.

johschmitz commented 3 years ago

My first feedback:

s = scenario_generator()
road = xodr.create_road(...)
s.add_road(road)
action = xosc.create_action()
s.add_actions(action)
s.generate_single_scenario(path_to_directory)

Seems simpler to me and easier to embed into other applications. To be honest this is not a blocker for me but felt a bit strange and makes your example code quite bloated and harder to understand.

mander76 commented 3 years ago

the ScenarioGenerator inheritance class is a way to parametrize scenarios and roads (and linking them together). To do this from an API level with "add_" functionality in the scenariogenerator will be very tricky. Baiscally the "road"/"action" in your example code snippet has to be related to parameters somehow (some lambda functionality maybe?), which I believe will be even more confusing for the user.