Skip to content

Using regex package for match #408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 15 commits into from
Jan 31, 2023
Merged

Using regex package for match #408

merged 15 commits into from
Jan 31, 2023

Conversation

kthyng
Copy link
Contributor

@kthyng kthyng commented Jan 30, 2023

The built-in re package does not allow for global flags like "(?i)" to be anywhere but the start of a pattern string now. The package regex still allows this, so it is optionally used for the match function if available.

  • test added
  • updating whats new

kthyng added 2 commits January 30, 2023 17:45
@kthyng
Using regex package for match
5d50976 
The built-in re package does not allow for global flags like "(?i)" to be anywhere but the start of a pattern string now. The package `regex` still allows this, so it is optionally used for the `match` function if available.
* test added
* updating whats new
@kthyng
Update whats-new.rst
61ce007
@kthyng kthyng requested a review from dcherian January 30, 2023 23:47
@kthyng kthyng linked an issue Jan 30, 2023 that may be closed by this pull request
Can we switch re to regex? #407
Closed
@kthyng
Copy link
Contributor Author

kthyng commented Jan 30, 2023

@dcherian How do I set up a test that will work for both with the optional dependences and without?

Also I wasn't sure which environment file to add regex to.

dcherian
dcherian reviewed Jan 31, 2023
View reviewed changes
dcherian
dcherian reviewed Jan 31, 2023
View reviewed changes
@dcherian
Copy link
Contributor

Can you add some docs under custom criteria too please? Also we should regex as an optional dependency in pyroject.toml

@dcherian
Copy link
Contributor

You'll need to add regex to ci/environment.yml too.

@kthyng
Copy link
Contributor Author

kthyng commented Jan 31, 2023

@dcherian I looked at the custom criteria docs and they are surprisingly good! Any specific topics you want me to add?

@dcherian
Copy link
Contributor

dcherian commented Jan 31, 2023 •
edited
Loading

Just that installing regex lets you do more complicated things, with a demo

@kthyng
Copy link
Contributor Author

kthyng commented Jan 31, 2023

I can demo a more complicated custom criteria — I have been using a similar set up for awhile though, it's just that re deprecated part of what made it possible, so it's not a new thing.

Any idea for how to address the mypy error? My idea let to the same error as before.

dcherian
dcherian reviewed Jan 31, 2023
View reviewed changes
dcherian
dcherian reviewed Jan 31, 2023
View reviewed changes
dcherian
dcherian reviewed Jan 31, 2023
View reviewed changes
@dcherian dcherian enabled auto-merge (squash) January 31, 2023 17:37
@dcherian
Copy link
Contributor

LGTM. Thanks!

@kthyng @dcherian
Update doc/custom-criteria.md
1e748a7 
Co-authored-by: Deepak Cherian <[email protected]>
dcherian
dcherian reviewed Jan 31, 2023
View reviewed changes
@kthyng
Copy link
Contributor Author

kthyng commented Jan 31, 2023

@dcherian should I do a next step on this or will you?

Also would it be possible to have a subsequent new release associated with this so I can use it in packages that use cf-xarray?

@dcherian dcherian disabled auto-merge January 31, 2023 17:53
@dcherian dcherian merged commit 368181c into xarray-contrib:main Jan 31, 2023
@kthyng kthyng deleted the regex branch January 31, 2023 18:05
dcherian added a commit to Descanonge/cf-xarray that referenced this pull request Feb 22, 2023
@dcherian
Merge branch 'main' into feat/flags-masks
a825a96 
* main: (33 commits)
  Update README.rst
  Update README.rst
  v0.8.0 release (xarray-contrib#424)
  Add sgrid axes parsing (xarray-contrib#421)
  Add rich repr (xarray-contrib#409)
  add `cf.grid_mapping_names` (xarray-contrib#391)
  Update CF standard name table v80 (xarray-contrib#423)
  Support grid_topology, mesh_topology CF roles. (xarray-contrib#420)
  added degrees units (xarray-contrib#390)
  Test and support 3.11 (xarray-contrib#417)
  Update link to COSIMA tutorial (xarray-contrib#419)
  Bump mamba-org/provision-with-micromamba from 14 to 15 (xarray-contrib#418)
  Update whats-new.rst (xarray-contrib#414)
  [skip-ci] Include data folder (xarray-contrib#416)
  Update changelog URL
  updated whats new for release (xarray-contrib#413)
  Using regex package for match (xarray-contrib#408)
  Try pytest-pretty (xarray-contrib#410)
  Update CITATION.cff (xarray-contrib#399)
  Fix upstream-dev CI (xarray-contrib#406)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcherian dcherian dcherian approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can we switch re to regex?
2 participants
@kthyng @dcherian