On Wed, Aug 22, 2018 at 9:13 AM Corentin CADIOU <corentin.cadiou@iap.fr> wrote:

Thank you all for your valuable feedback.

I agree with Nathan that we should keep it on the master branch for the time being, at least until the feature is stable. I've got a working version for python 3 (see https://github.com/yt-project/yt/pull/1931) but not for python 2 (because of annoying string issues) that includes your comments. Don't hesitate to kick-in and suggest improvement / comments.


Thankfully the yt-4.0 branch will only target python3 on release. So far I've kept testing on python2.7 going to make backporting easier but it's starting to cause issues for developers who need to hack things to get them to work on python2.7.

In fact, how about today I issue a pull request to turn off testing on python2 on that branch? :)
 

Corentin

On 22/08/2018 15.18, John ZuHone wrote:
I concur with this. I think the per-field configuration is a great idea!

On Aug 22, 2018, at 9:14 AM, Nathan Goldbaum <nathan12343@gmail.com> wrote:

one more thing:

I think we shouldn’t merge this until 3.5 is out the door and we should target this new config file format for the yt 4.0 release. I think we’re too late in the cycle for 3.5 to get this in for the 3.x series without risking major user pain. Best to test it out for at leasy a few months on the master branch before a release.

On Tue, Aug 21, 2018 at 11:19 AM Nathan Goldbaum <nathan12343@gmail.com> wrote:


On Tue, Aug 21, 2018 at 11:00 AM Cameron Hummels <chummels@gmail.com> wrote:
Interesting idea--I'm all for allowing more configurability in the config file on disk.  I guess I don't understand the problem with leaving it as a YAML file and adding in additional configuration within that framework?  Is it just that it doesn't allow for subsections? 

Not sure what you mean by this. We're not using YAML currently.

We *could* switch to YAML instead of TOML (or some other format) but I think that would be a mistake as summarized in this PEP:


YAML has lots of issues and corner cases, best to avoid that if we have the freedom to do so.
 
Looking forward to seeing how this all turns out!

Cameron

On Tue, Aug 21, 2018 at 7:37 AM, Matthew Turk <matthewturk@gmail.com> wrote:
As a quick update, I looked at the usages of ytcfg, and it looks like
in the 43 files that it shows up, the most common things it checks:

 * default colormap
 * communicator information for parallelism
 * if we're in testing or not

The latter two are basically using ytcfg to pass around globals that
are set up internally in yt.  The others seem related to xray data,
test data, etc.

My *guess* is that even if it's not sed-able, this wouldn't be an
overwhelming thing to change.  But, it would require a bit of work.
On Tue, Aug 21, 2018 at 9:30 AM Nathan Goldbaum <nathan12343@gmail.com> wrote:
>
>
>
> On Tue, Aug 21, 2018 at 8:12 AM Matthew Turk <matthewturk@gmail.com> wrote:
>>
>> Hi Corentin,
>>
>> Great discussion.
>>
>> On Tue, Aug 21, 2018 at 4:34 AM Corentin CADIOU <corentin.cadiou@iap.fr> wrote:
>> >
>> > Hello,
>> >
>> > As of today, there is no way to configure yt plots per-field. One is stuck with configuration that are yt-wide (e.g. default_colormap). For example, I often find myself typing p.set_cmap('density', 'dusk'). I suggest to add a way to configure the behaviour of yt plotting interface per-field using yt configuration, so that I could set once and for all some defaults for plots. I pushed a first PR on github (https://github.com/yt-project/yt/pull/1931) and opened an issue (https://github.com/yt-project/yt/issues/1888) to discuss the feature.
>> >
>> > Nathan and I discussed a bit about how this should be implemented in the configuration file. Since ini files are quite restricted (e.g. you cannot have subsections), Nathan suggested at one point to use the toml format (https://github.com/toml-lang/toml). However, it is incompatible with the older .ini format. For example, boolean should be in lowercase (true, false) and not Python-like (True, False), strings have to be surrounded by apostrophes, ...
>>
>> Taking a step back, I suspect that the following are both true:
>>
>> 1) Folks who have been using yt for a lot longer are more likely to
>> have configuration files scattered about
>> 2) The existing config files don't actually do *that* much
>>
>> We've also migrated in the past, and could probably implement that
>> again.  (Kacper did it the first time.)
>
>
> This is referring to "yt config migrate" which you should take a look at. We could definitely overload the existing command for this purpose.
>
> I think using the standard library configparser module you could first load the ini-style file, inspect the state of the resulting ConfigParser object, convert the data to a dict, and pass that dict to a toml writer.
>
>>
>> So I don't think that this
>> should be that big of a barrier; we could also consider the idea of
>> having a field configuration toml file that sits next to the ini file.
>> Do we need new dependencies if we have toml?
>
>
> We'd probably want to depend on either this:
>
> https://github.com/uiri/toml
>
> which seems to be the most popular library or this:
>
> https://github.com/avakar/pytoml
>
> Which seems to be what pip has vendored and I trust dstufft's opinion on these things...
>
> Another option would be to vender one of those libraries but I don't think that's necessary anymore given the state of the packaging ecosystem and that both of these are pure python.
>
>>
>> >
>> > Another suggestion is to add each fields' configuration as a new section in the ini file. For example
>> >
>> > [config.field_type.field_name]
>> > cmap = viridis
>> > log = False
>> >
>> > This is what is implemented in the PR, but maybe some of you have more clever ideas about this? One fairly strong constraint is that the configuration files should be as backward-compatible as possible. If not, we'd have to write a migration script from the old to the new format.
>>
>> Or have them be in a separate file, which might even make sense -- and
>> would be consistent with the idea that we may eventually want to
>> override these by frontend.
>
>
> This is starting to get close to my ideas for field_day. I was also hoping to use toml there...
>
> But yes, it would be good to use whatever syntax we come up with here for frontend-specific overrides.
>
>> >
>> > We should also discuss what can be configured here. There is already the colormap and log scale implemented. One very neat feature would be to be able to configure the default unit, but it may be hard to implement (e.g. how do we configure the default unit for both projection and slices?) and probably confusing for the user.
>> >
>>
>> Definitely default unit for non-integrated quantities!  I think for
>> integrated quantities like projections what we could do is configure
>> the default integration unit, and that would combine with the default
>> unit to produce the final result.
>
>
> Yup, and we need to be clear to document this use case since it'll come up a lot.
>
>>
>>
>> > So here are the questions. Do you think per-field configuration should be part of yt? If so what format should it use? And what are your thoughts on what should and shouldn't be configurable?
>>
>> 1) Yes
>> 2) I don't particularly mind toml, but I'd rather we break hard if
>> we're going to break from INI-style files.  A second file would make
>> sense to me.
>
>
> I agree that this is a worthwhile improvement. I would also like to move to a richer configuration format than ini-style files. ConfigParser is also a pain to deal with.
>
> That said, unless you also want to systematically update everywhere we use the configuration system in yt (might be possible with some sed magic, so don't discount just updating everything) you'd also need to write an emulation layer or figure out how to generate a ConfigParser object from the toml file.
>
>>
>>
>> >
>> > Corentin
>> >
>> > --
>> > Corentin Cadiou
>> > PhD student
>> > Institut d'Astrophysique de Paris (IAP), desk 142b
>> > 98 bis boulevard Arago
>> > 75014 Paris, France
>> >
>> > phone: +33.6.43.18.66.83
>> >
>> > _______________________________________________
>> > yt-dev mailing list -- yt-dev@python.org
>> > To unsubscribe send an email to yt-dev-leave@python.org
>> _______________________________________________
>> yt-dev mailing list -- yt-dev@python.org
>> To unsubscribe send an email to yt-dev-leave@python.org
>
> _______________________________________________
> yt-dev mailing list -- yt-dev@python.org
> To unsubscribe send an email to yt-dev-leave@python.org
_______________________________________________
yt-dev mailing list -- yt-dev@python.org
To unsubscribe send an email to yt-dev-leave@python.org



--
Cameron Hummels
NSF Postdoctoral Fellow
Department of Astronomy
California Institute of Technology
_______________________________________________
yt-dev mailing list -- yt-dev@python.org
To unsubscribe send an email to yt-dev-leave@python.org
_______________________________________________
yt-dev mailing list -- yt-dev@python.org
To unsubscribe send an email to yt-dev-leave@python.org


_______________________________________________
yt-dev mailing list -- yt-dev@python.org
To unsubscribe send an email to yt-dev-leave@python.org
-- 
Corentin Cadiou
PhD student
Institut d'Astrophysique de Paris (IAP), desk 142b
98 bis boulevard Arago
75014 Paris, France

phone: +33.6.43.18.66.83
_______________________________________________
yt-dev mailing list -- yt-dev@python.org
To unsubscribe send an email to yt-dev-leave@python.org