Skip to content
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.

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

Expose Python API as package imports #352

Merged
merged 3 commits into from
Jan 8, 2025
Merged

Conversation

kieran-ryan
Copy link
Member

@kieran-ryan kieran-ryan commented Jan 8, 2025

🤔 What's changed?

  • Exposed Python API as package imports
- from gherkin.parser import Parser
- from gherkin.pickles.compiler import Compiler
+ from gherkin import Compiler, Parser
  • Dropped count_symbols_test.py unit test module

⚡️ What's your motivation?

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

  • Whether anything should additionally be exposed; or indeed reverted

📋 Checklist:

- Python 2 compatibility character count code has been removed
@kieran-ryan kieran-ryan added the ⚡ enhancement Request for new functionality label Jan 8, 2025
@kieran-ryan kieran-ryan self-assigned this Jan 8, 2025
@kieran-ryan kieran-ryan marked this pull request as ready for review January 8, 2025 21:19
@kieran-ryan kieran-ryan requested review from youtux and jsa34 January 8, 2025 21:20
@kieran-ryan kieran-ryan changed the title Expose Python public API as package imports Expose Python API as package imports Jan 8, 2025
python/gherkin/__init__.py Outdated Show resolved Hide resolved
@kieran-ryan kieran-ryan force-pushed the feat/py-package-imports branch from c8ad406 to 8398501 Compare January 8, 2025 22:05
@kieran-ryan kieran-ryan merged commit 5319c98 into main Jan 8, 2025
6 checks passed
@kieran-ryan kieran-ryan deleted the feat/py-package-imports branch January 8, 2025 22:14
@kieran-ryan
Copy link
Member Author

kieran-ryan commented Jan 8, 2025

Thank for review @youtux - and similarly @jsa34 (saw you left and dropped comment) - appreciate it both!

@jsa34
Copy link
Contributor

jsa34 commented Jan 9, 2025

I dropped it because I thought I was being too picky ( apologies! 🫣)

@kieran-ryan
Copy link
Member Author

I dropped it because I thought I was being too picky ( apologies! 🫣)

I actually would have made same comment @jsa34! 😄 Appreciate raising! Is interesting. The way I would have formatted __all__ was as you described - alphabetically; though with ruff's automatic fix for sorting __all__ - RUF022 (unsorted-dunder-all) - it performs a natural sort, respecting case first before alphabetic ordering. Something we can format and validate - which is cool; though perhaps not the 'go to' style for many.

__all__ = [
    "a_variable",
    "CapWords",
    "UPPER_CASE",
]
ruff check --select=RUF022 module.py --fix
__all__ = [
    "UPPER_CASE",
    "CapWords",
    "a_variable",
]

@jsa34
Copy link
Contributor

jsa34 commented Jan 10, 2025

Ah! Never knew - every day's a school day! Thanks for the detailed reply!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡ enhancement Request for new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants