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

Allow for the Specification of Either XPath Version 1 or XPath Version 2 When Running Checks #15

Closed
gkapfham opened this issue Aug 24, 2023 · 12 comments · Fixed by #90
Closed
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed in-progress Work is actively happening on this issue

Comments

@gkapfham
Copy link
Collaborator

The current implementation of chasten is hard-coded so that it only works with
XPath 2 expressions. While this is nice because of the fact that it supports the
greatest range of XPath expressions, it is likely going to make the execution of
XPath expressions slower when they would work sufficiently well with the version
1 system instead of the version 2 system.

There are some more details about this issue at:

https://github.com/spookylukey/pyastgrep

One idea would be that chasten analyze could accept a command line argument
that would support the specification of whether or not the tool should use XPath
1 or XPath 2 to run all of the expressions associated with the checks.

Alternatively, it might be a good idea to allow the person who writes the
checks.yml file to use an extra attribute to specify whether a certain check
should use the XPath version 1 or version 2 parse.

@gkapfham
Copy link
Collaborator Author

FIXME: add finley to this

@gkapfham gkapfham added the in-progress Work is actively happening on this issue label Sep 12, 2023
@VitalJoseph VitalJoseph self-assigned this Sep 12, 2023
@simojo simojo added this to the Week 4 Features milestone Sep 12, 2023
@Finley8 Finley8 self-assigned this Sep 15, 2023
@Finley8 Finley8 changed the title Allow for the Specification of Either XPath Version 1 or XPath Version 2 When Running Checks WIP: Allow for the Specification of Either XPath Version 1 or XPath Version 2 When Running Checks Sep 15, 2023
@KevenDuverglas KevenDuverglas self-assigned this Sep 17, 2023
@KevenDuverglas
Copy link
Collaborator

Myself (@Finley8), @gyamfi01, and I reviewed the checks.yml file. As we were trying to understand the differences between XPath 2 expressions and XPath 1 expressions, we found it challenging to decipher the distinction. We're aware that checks.yml is essential for the functionality of chasten. We have the following questions:

  1. How can you determine that XPath 2 is slower than XPath 1?
  2. Were specific tests run to see the differences in performance between XPath 1 and XPath 2 expressions?
  3. Which resources did @gkapfham use to write the checks.yml file? Were there any particular blueprints or online websites referenced?
  4. Is there an issue with chasten, or is this just a suggestion to improve its usage?
  5. What influenced the decision to use XPath 2 over XPath 1 for the checks.yml file?

@Finley8
Copy link
Collaborator

Finley8 commented Sep 17, 2023

@Finley8 (Me), @gyamfi01, and @KevenDuverglas researched Xpath 1 and 2 expressions. We focused on the relations of syntax and nodes within XPath expressions. We have searched through the checks.yml file to determine what must be changed. We could not figure out how to add or change the checks.yml file to implement Xpath 1. We need to have more conversations with the lead developer to figure out how much more time we will need to spend on this issue.

1 similar comment
@Finley8
Copy link
Collaborator

Finley8 commented Sep 17, 2023

@Finley8 (Me), @gyamfi01, and @KevenDuverglas researched Xpath 1 and 2 expressions. We focused on the relations of syntax and nodes within XPath expressions. We have searched through the checks.yml file to determine what must be changed. We could not figure out how to add or change the checks.yml file to implement Xpath 1. We need to have more conversations with the lead developer to figure out how much more time we will need to spend on this issue.

@VitalJoseph
Copy link
Collaborator

Myself (@Finley8), @gyamfi01, and I reviewed the checks.yml file. As we were trying to understand the differences between XPath 2 expressions and XPath 1 expressions, we found it challenging to decipher the distinction. We're aware that checks.yml is essential for the functionality of chasten. We have the following questions:

  1. How can you determine that XPath 2 is slower than XPath 1?
  2. Were specific tests run to see the differences in performance between XPath 1 and XPath 2 expressions?
  3. Which resources did @gkapfham use to write the checks.yml file? Were there any particular blueprints or online websites referenced?
  4. Is there an issue with chasten, or is this just a suggestion to improve its usage?
  5. What influenced the decision to use XPath 2 over XPath 1 for the checks.yml file?

To answer question 1, XPath 2 introduced a lot of additional features and improvements like more data types, higher order funtions, and more built in-functions. With these new implementations, Xpath 2 is able to write and evaluate more complex expressions that cant be written in XPath 1. These features may result in a slower runtime for XPath 2 compared to Xpath 1.

@simojo simojo removed their assignment Sep 19, 2023
@gkapfham gkapfham changed the title WIP: Allow for the Specification of Either XPath Version 1 or XPath Version 2 When Running Checks Allow for the Specification of Either XPath Version 1 or XPath Version 2 When Running Checks Sep 22, 2023
@gkapfham gkapfham added the enhancement New feature or request label Sep 22, 2023
@KevenDuverglas
Copy link
Collaborator

@VitalJoseph, @Finley8, @KevenDuverglas, and @gyamfi01 Did research and tested the xpath1 on chasten and we discovered to the best of our knowledge that the pyastgrep tool doesn't have xpath1 embedded in it. Here is more documentation:

  • https://github.com/spookylukey/pyastgrep/blob/master/src/pyastgrep/search.py, upon looking at this Python file my group came to the discovery that a new tool might be needed if we were to test xpath1. Not only that but the main.py file in chasten would need to be altered significantly, there needs to be a new tool implemented to service xpath1.
  • And there is much more to ask regarding this issue and whether it should be closed.

@Finley8 Finley8 added the help wanted Extra attention is needed label Sep 27, 2023
@simojo simojo self-assigned this Sep 30, 2023
@simojo
Copy link
Collaborator

simojo commented Sep 30, 2023

@simojo
Copy link
Collaborator

simojo commented Sep 30, 2023

@VitalJoseph, @Finley8, @KevenDuverglas, and @gyamfi01 Did research and tested the xpath1 on chasten and we discovered to the best of our knowledge that the pyastgrep tool doesn't have xpath1 embedded in it. Here is more documentation:

  • https://github.com/spookylukey/pyastgrep/blob/master/src/pyastgrep/search.py, upon looking at this Python file my group came to the discovery that a new tool might be needed if we were to test xpath1. Not only that but the main.py file in chasten would need to be altered significantly, there needs to be a new tool implemented to service xpath1.
  • And there is much more to ask regarding this issue and whether it should be closed.

Just dropping this here for reference: within the file you mentioned, you can specify XPath version here

@simojo
Copy link
Collaborator

simojo commented Sep 30, 2023

@KevenDuverglas @Finley8 @VitalJoseph @gyamfi01 Is there any kind of feature branch with what's been done so far for this issue? If not, let's make one as soon as we can and we'll start messing with the config schema for starters. @ me when you can.

@Finley8
Copy link
Collaborator

Finley8 commented Oct 2, 2023

@KevenDuverglas
Copy link
Collaborator

KevenDuverglas commented Oct 3, 2023

While running chasten we came upon an error where you are unable to see new command line interfaces unless you run poetry run first.

@Finley8
Copy link
Collaborator

Finley8 commented Oct 3, 2023

Analyze 1
Analyze 2
Here is our problem when run. Do you have any ideas on how we can fix this? You can see that --xpath shows up in the second screenshot but not the first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed in-progress Work is actively happening on this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants