catkin / xylem

A tool for resolving dependencies in a platform agnostic way.
Apache License 2.0
1 stars 1 forks source link

Solution for "python3" as OS 'features' #13

Open NikolausDemmel opened 10 years ago

NikolausDemmel commented 10 years ago

Here is a proposal for how to realize this: http://xylem.readthedocs.org/en/latest/design.html#os-options

This also includes a discussion why using "derivative OS" is not a good choice IMO.

Feel free to comment / discuss in this issue.

NikolausDemmel commented 10 years ago

cc @wjwwood @tfoote @dirk-thomas @esteve

tfoote commented 10 years ago

We need to make sure that the expanded format is also writable and readable without using the compact notation. And using the ordering of triplets will fail with a standard alphabetical export. I'd suggest using a few more characters while using a special prefix something like feature_true_python3 feature_false_python3. The exact prefix could probably find better wording, but if that specific prefix is encountered it causes the resolution to descend into the subtrees.

Also since we're possibly doing the same key munging as #10 so maybe feature==python3, feature!=python3 and use the same syntax and parsing.

NikolausDemmel commented 10 years ago

Your whole comment including the last bit suggesting feature==python3 is about the expanded version, right?

Yes, i fully agree that the expanded version needs to be write/readable as well. The idea with rules expansion is that it transforms shorthand notation suitable for humans to a more explicit form suitable for lookup. But rules expansion should be idempotent and valid YAML that can be written and read.

Unfortunately when I made this post I messed up with updating read-the-docs and it was actually not linking to the latest version of that text. It is fixed now and I have made slight adjustments to the internal format to use dicts instead of ordered lists. Does that address your concern? (The examples in the text don't actually have alphabetical ordering for the dicts, but the order for dicts does not matter anyway). Also, for the shorthand format the negative !python3 have been removed as they are redundant.

Anyway with the previous format I don't think the ordering would have been a problem, since the triplets were supposed to be lists, which retain their order. I also did not want to introduce names such as feature_true_python3 or feature==python3 in the expanded format since I want to avoid string parsing during lookup, i.e. use only string equality / hashing.

One point regarding using only yaml.load and yaml.write for a simple script on: It cannot really be done reliably, simply for the fact that unicode handling is not fully seemless with python yaml. Also our file format needs some customization like not writing lists of packages as multi-line lists. I currently use these helpers. Nevertheless I agree that we should not introduce non-alphabetical ordering.

dirk-thomas commented 10 years ago

Regarding clean unicode support: you might want to make all code use Python 3 by default.

Making it work for Python 2 afterwards is much easier then the other way around.

NikolausDemmel commented 10 years ago

@dirk-thomas: At the moment I'm (trying to) write code that works on py2 and py3. In particular tests are run for both (testsuite is not so big yet, though). The strategy regarding unicode is representing all text data internally as unicode/str on py2/py3.

dirk-thomas commented 10 years ago

I was referring to e.g. https://github.com/catkin/xylem/blob/ab0c4a4dd0bbae5da7f266c2c5765d8da7b699f7/scripts/xylem#L1

I usually find it easier to make Python 3 the default (for new development) since it is much more "picky" when it comes to bytes / str when dealing with encode / decode.

NikolausDemmel commented 10 years ago

Ah ok. Yes, maybe it makes sense to do local development/testing with python 3, however I don't actually use that script but rather the setuptools generated script, which should use the correct verison when installed with python3 setup.py develop.

I think for a helper script such as the one you pointed out it makes sense reference the default python, which is one of 2 or 3 on most current systems, I guess. (And the way it is it also works with virtualenv regardless of python version, I assume). But I assume your point was not to change that script but to encourage me to use py3 for development.

By the way tools like python-modernize help to write py2/3 compatible code, e.g. by making sure you have from __future__ import unicode_literals where needed. I have not had trouble after switching to the strategy of keeping all text as unicode/str and doing encode/decode explicitely on I/O.

tfoote commented 10 years ago

Yes, my whole comment was about the expanded version. The dictionary approach works too. Though when exported the feature will alphabetize to the middle. ​active/inactive seems reasonable.