Make sampler_type required for built-in derived fields.

#2385 Merged at 9b5937a
Repository
al007
Branch
yt
Repository
yt_analysis
Branch
yt
Author
  1. Alexander Lindsay
Reviewers
Description

Built-in derived fields require sampler_type to be specified. User fields created with ds.add_field, however, will default to sampler_type = "cell" unless otherwise specified.

Comments (3)

  1. yt-fido
  2. Nathan Goldbaum

    I chatted with Alex about this in person, but commenting here so we don't lose track. Right now this PR modifies the signature of add_field so that sampling_type is a required argument. While in the future we might want to make this change, I don't think we can right now due to backward compatibility concerns. In particular, the way this is currently implemented would likely break uses of yt.add_field in existing user scripts due to the change in the argument order.

    For now, I think sampling_type needs to remain an optional keyword argument of yt.add_field, but we should make it required for uses of yt.add_field that are completely internal to yt.

    In particular, I think the changes to field_info_container.py are fine, but that we should take care to ensure uses of the yt.add_field (which is really LocalFieldInfoContainer.add_field) still continue to work, possibly while issuing a warning that sampling_type will become required in the future if a user doesn't specify it.

  3. Alexander Lindsay author

    Thanks for reviewing Andrew. I think I've now caught the places where my naive find and replace created contradicting definitions of 'sampling_type' and 'particle_type'.