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

406 support enabling/disabling particular atoms #426

Merged
merged 35 commits into from
Jul 19, 2024

Conversation

deemp
Copy link
Member

@deemp deemp commented Jul 1, 2024

Issues


PR-Codex overview

This PR updates the eo-phi-normalizer package by adding new phi files, updating pipeline configurations, and introducing new rules.

Detailed summary

  • Added new phi files for error, dataized, cti, try, rust, string, cage, true, false, nan, go, and while
  • Updated pipeline configurations and hashes
  • Modified TH.hs, Config.hs, RunYegor.hs, Fast.hs
  • Introduced new rules and functions in various files

The following files were skipped due to too many changes: eo-phi-normalizer/data/0.38.4/org/eolang/while.phi, eo-phi-normalizer/data/0.38.4/org/eolang/seq.phi, eo-phi-normalizer/data/0.38.4/org/eolang/int.phi, eo-phi-normalizer/data/0.38.4/org/eolang/switch.phi, eo-phi-normalizer/src/Language/EO/Phi/Pipeline/Dataize/PrintConfigs.hs, eo-phi-normalizer/data/0.38.4/org/eolang/malloc.phi, eo-phi-normalizer/data/0.38.4/org/eolang/bytes.phi, eo-phi-normalizer/src/Language/EO/Phi/Rules/Common.hs, eo-phi-normalizer/data/0.38.4/org/eolang/tuple.phi, eo-phi-normalizer/data/0.38.4/org/eolang/float.phi, site/docs/src/pipeline.md, scripts/pipeline.sh, eo-phi-normalizer/data/0.38.4/org/eolang/positive-infinity.phi, eo-phi-normalizer/data/0.38.4/org/eolang/negative-infinity.phi, eo-phi-normalizer/eo-phi-normalizer.cabal, eo-phi-normalizer/app/Main.hs, eo-phi-normalizer/src/Language/EO/Phi/Dataize.hs

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@deemp deemp linked an issue Jul 1, 2024 that may be closed by this pull request
6 tasks
@deemp deemp requested a review from fizruk July 1, 2024 18:16
fail (name <> ": Couldn't find bytes in RHS: " <> printTree (hideRho r))
return (result, ())
ctx <- getContext
if HashSet.member (DisabledAtomName name) ctx.disabledAtomNames
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not the correct place for this check. Instead, use the check in the dataizeStepChain function, before evaluateBuiltinFunChain is called.

@0crat
Copy link

0crat commented Jul 6, 2024

@fizruk Thanks for the review! You've earned +25 points for this.

@deemp
Copy link
Member Author

deemp commented Jul 10, 2024

To enable or disable particular atoms for a pipeline test set, add an atoms object into the test set in the pipeline config (the file path is pipeline/config.yaml).

If you enable particular atoms, only these atoms are enabled.
If you don't enable particular atoms, all atoms except disabled are enabled.

Here's an example of a test set where the foo atom is enabled and the bar atom is disabled.
Note that the Package atom is disabled.

- eo:
    original: eo/eo-runtime/src/test/eo/org/eolang/as-phi-tests.eo
    yaml: pipeline/eo-yaml/as-phi-tests.yaml
    filtered: pipeline/eo-filtered/as-phi-tests.eo
  phi:
    initial: pipeline/phi-initial/as-phi-tests.phi
    normalized: pipeline/phi-normalized/as-phi-tests.phi
    bindings-path-initial: org.eolang
    bindings-path-normalized: org.eolang
  atoms:
    enable:
      - foo
    disable:
      - bar

@fizruk fizruk changed the title 406 support enablingdisabling particular atoms 406 support enabling/disabling particular atoms Jul 11, 2024
@deemp deemp force-pushed the 406-support-enablingdisabling-particular-atoms branch from eb93f5b to ffe4119 Compare July 11, 2024 16:53
@deemp deemp mentioned this pull request Jul 11, 2024
2 tasks
@deemp deemp requested a review from fizruk July 16, 2024 11:34
ctx <- getContext
return (ctx, Left obj')
ctx' <- getContext
if HashSet.member funcName knownAtomNames && not (HashSet.member funcName ctx'.enabledAtomNames)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would factor this condition into a separate function.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 32be6c9

However, I'm not sure I understood your suggestion correctly.

@@ -270,87 +278,129 @@ evaluateBytesBytesFunChain name = evaluateUnaryDataizationFunChain name intToByt
evaluateFloatFloatFloatFunChain :: String -> (Double -> Double -> Double) -> Object -> EvaluationState -> DataizeChain (Object, EvaluationState)
evaluateFloatFloatFloatFunChain name = evaluateBinaryDataizationFunChain name floatToBytes bytesToFloat wrapBytesInFloat extractRho (extractLabel "x")

knownAtoms :: [(String, String -> Object -> EvaluationState -> DataizeChain (Object, EvaluationState))]
knownAtoms =
[ ("Lorg_eolang_int_gt", \name obj -> evaluateIntIntBoolFunChain name (>) obj)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you change the order of arguments, you can avoid lambdas and simply write evaluateIntIntBoolFunChain (>).

Copy link
Member Author

@deemp deemp Jul 17, 2024

Choose a reason for hiding this comment

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

Addressed in 9653b2e


evaluateBuiltinFunChainUnknown :: String -> a -> b1 -> Chain (Either a b2) (a, b1)
evaluateBuiltinFunChainUnknown atomName obj state = do
logStep [fmt|[WARNING]: unknown atom ({atomName})|] (Left obj)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unknown atoms are normal and should not be warnings.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 75fb769

@deemp deemp requested a review from fizruk July 17, 2024 22:13
@fizruk fizruk merged commit c2ce147 into master Jul 19, 2024
8 checks passed
@fizruk fizruk deleted the 406-support-enablingdisabling-particular-atoms branch July 19, 2024 03:43
@0crat
Copy link

0crat commented Jul 19, 2024

@deemp Thanks for the contribution! You've earned +5 points for this: +20 as a basis; +5 for 2236 hits-of-code; -7 for too many hits-of-code (2236 >= 100); -15 for way too many hits-of-code (2236 >= 400); -9 for 9 comments; +11 to give you at least something. Please, keep them coming.

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

Successfully merging this pull request may close these issues.

Support enabling/disabling particular atoms
3 participants