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

REF/ENH: subclass TaoCore with interface methods, add special parsers #75

Merged
merged 16 commits into from
Jun 27, 2024

Conversation

ken-lauer
Copy link
Collaborator

@ken-lauer ken-lauer commented Jun 13, 2024

Changes

  • This makes the Tao class that users see have interface methods
  • This should allow for VSCode and other editors with language servers see the interface methods on the Tao object
  • Removed runtime method patching
  • Adds pre-commit configuration for checking the codebase; follow-up PR will do some reformatting if the maintainers are amenable
  • Additionally, closes Add logger #29 by adding a logger.exception in the noted spot

Context

I was hoping to get method tab completion in my language server-enabled editor (VSCode, vim + lsp, etc. should all be the same here); interactive Jupyter/IPython have no issue with the current implementation because the instance already exists with the patched-in methods.

Copy link
Collaborator

@hhslepicka hhslepicka left a comment

Choose a reason for hiding this comment

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

I like this change and how it was implemented.
Thank you for working on that!

@ken-lauer ken-lauer marked this pull request as ready for review June 13, 2024 17:39
@ken-lauer ken-lauer mentioned this pull request Jun 17, 2024
@hhslepicka hhslepicka closed this Jun 19, 2024
@hhslepicka hhslepicka reopened this Jun 19, 2024
@hhslepicka hhslepicka closed this Jun 19, 2024
@hhslepicka hhslepicka reopened this Jun 19, 2024
@ChristopherMayes
Copy link
Contributor

The tests run fine for me locally.

@ken-lauer
Copy link
Collaborator Author

ken-lauer commented Jun 20, 2024

@ChristopherMayes should I make CI changes here (installing bmad from conda) or would you prefer it in a separate PR?

I'll just put in a separate PR for cleanliness

@ChristopherMayes
Copy link
Contributor

ChristopherMayes commented Jun 25, 2024

I see this in the basic notebook:

from pytao import Tao

tao=Tao('-init $ACC_ROOT_DIR/bmad-doc/tao_examples/cesr/tao.init -noplot')   

names = tao.lat_list('*', 'ele.name')

Failed to parse string data. Returning raw value.
Traceback (most recent call last):
  File "[...pytao/pytao/interface_commands.py", line 68](http://localhost:8889/pytao/interface_commands.py#line=67), in __execute
    data = parse_tao_python_data(ret)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[.../pytao/pytao/tao_ctypes/util.py", line 143](http://localhost:8889/pytao/tao_ctypes/util.py#line=142), in parse_tao_python_data
    dat.update(parse_tao_python_data1(l, clean_key))
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "[.../pytao/pytao/tao_ctypes/util.py", line 126](http://localhost:8889/pytao/tao_ctypes/util.py#line=125), in parse_tao_python_data1
    name, type, setable = sline[0:3]
    ^^^^^^^^^^^^^^^^^^^
ValueError: not enough values to unpack (expected 3, got 1)

@ChristopherMayes
Copy link
Contributor

I tried manually adding a test, which gives the same error when calling directly, but oddly pytest doesn't raise anything

def test_lat_list_3():
    tao = Tao("-init $ACC_ROOT_DIR/regression_tests/python_test/cesr/tao.init -noplot")
    
    tao.lat_list(
        ix_uni="1", ix_branch="0", elements="Q*", which="design", who="ele.name"
    )    

@ken-lauer
Copy link
Collaborator Author

ken-lauer commented Jun 26, 2024

but oddly pytest doesn't raise anything

It's not an exception but rather a log message, that's why it doesn't raise.

This is a failure of the parser - and the fallback is to return a raw string.
The code is here: https://github.com/ken-lauer/pytao/blob/fbdc8b7e3b0402e951b7eca3e9d56365a5b4d32d/interface.tpl.py?plain=1#L65-L66

Should we instead raise here? Or should we at least make the test suite aware of when this exception is logged?

Edit: if I fail the test on unsuccessful parsing, we have a lot of failures:

============================================================== short test summary info ==============================================================
FAILED pytao/tests/test_interface_commands.py::test_building_wall_list_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_building_wall_list_2 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_building_wall_graph_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_constraints_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_constraints_2 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_data_d1_array_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_data_d2_array_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_data_parameter_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_datum_has_ele_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_ele_chamber_wall_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_ele_elec_multipoles_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_ele_gen_grad_map_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_ele_lord_slave_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_ele_multipoles_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_ele_spin_taylor_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_ele_taylor_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_ele_wall3d_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_em_field_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_enum_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_floor_plan_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_floor_orbit_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_help_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_inum_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_lat_calc_done_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_lat_branch_list_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_lat_param_units_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_plot_lat_layout_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_plot_graph_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_plot_line_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_plot_symbol_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_shape_list_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_shape_pattern_list_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_show_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_species_to_int_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_species_to_str_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_spin_polarization_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_spin_resonance_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_super_universe_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_var_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_var_2 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_var_general_1 - Failed: Failed to parse string data. Returning raw value.:
FAILED pytao/tests/test_interface_commands.py::test_var_v1_array_1 - Failed: Failed to parse string data. Returning raw value.:
========================================================== 42 failed, 71 passed in 24.64s ===========================================================

@ken-lauer ken-lauer changed the title REF: subclass TaoCore with interface methods REF/ENH: subclass TaoCore with interface methods, add special parsers Jun 26, 2024
@ken-lauer
Copy link
Collaborator Author

ken-lauer commented Jun 27, 2024

I wrote a bunch of custom parsers this morning to address those failures.

Remaining to do:

@ken-lauer
Copy link
Collaborator Author

OK, @ChristopherMayes and @hhslepicka, this PR grew a bit out of proportion. There are no more tao commands silently failing parsing as of the last couple commits.

Mind taking another look so we can move on?

pytao/util/parsers.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@hhslepicka hhslepicka left a comment

Choose a reason for hiding this comment

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

Amazing work @ken-lauer! I just left a small suggestion to fix a typo (not on this PR) and a question about the __version__.

ken-lauer and others added 3 commits June 27, 2024 10:42
Co-authored-by: Hugo Slepicka <hhslepicka@users.noreply.github.com>
This reverts commit 28f37ad.

This appears to be working on CI (Linux) but failing locally for me
(MacOS) with bmad 20240626.0
@hhslepicka hhslepicka merged commit 5fb8e46 into bmad-sim:master Jun 27, 2024
4 checks passed
@ken-lauer ken-lauer deleted the ref_subclass_interface branch June 27, 2024 17:50
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.

Add logger
3 participants