Skip to content

Conversation

@neutrinoceros
Copy link
Member

PR Summary

I just learned about difflib.get_close_matches, and it seems like a nice way to remove a custom function of ours.

PR Checklist

  • New features are documented, with docstrings and narrative docs
  • Adds a test for any bugs fixed. Adds tests for new features.

@neutrinoceros neutrinoceros added this to the 4.5.0 milestone Jun 29, 2025
@neutrinoceros neutrinoceros added refactor improve readability, maintainability, modularity deprecation deprecate features or remove deprecated ones labels Jun 29, 2025
@neutrinoceros neutrinoceros requested a review from cphyc June 29, 2025 18:28
@neutrinoceros neutrinoceros force-pushed the rfc/ditch-levenshtein branch 5 times, most recently from 9581b01 to 1571b9b Compare June 29, 2025 19:24
@neutrinoceros neutrinoceros marked this pull request as ready for review June 29, 2025 20:04
Copy link
Member

@cphyc cphyc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are loosing some relevant suggestions (when there are more than 3 possible matches, try e.g. querying ad["index", "d"] (it should suggest x,y,z,dx,dy,dz at least. With the current PR, it only suggests the last three.

With the changes, I get more irrelevant (but also more relevant ones) suggestions:

ds = yt.load_sample("output_00080")
ad = ds.all_data()

ad["gas", "densty"]

Previously:

YTFieldNotFound: Could not find field ('gas', 'densty') in info_00080.
Did you mean:
	('gas', 'density')

This PR:

YTFieldNotFound: Could not find field ('gas', 'densty') in info_00080.
Did you mean:
	('gas', 'density')
	('gas', 'dy')
	('ramses', 'Density')

@neutrinoceros neutrinoceros force-pushed the rfc/ditch-levenshtein branch from 04710d5 to d5a152f Compare June 30, 2025 12:46
@neutrinoceros neutrinoceros requested a review from cphyc June 30, 2025 12:46
cphyc
cphyc previously approved these changes Jun 30, 2025
@neutrinoceros
Copy link
Member Author

rebased to resolve merge conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecation deprecate features or remove deprecated ones refactor improve readability, maintainability, modularity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants