Skip to content

Allow relative imports / functions in add/remove-atom#141

Open
rTreutlein wants to merge 2 commits intomainfrom
fix/relative-import&add-atom
Open

Allow relative imports / functions in add/remove-atom#141
rTreutlein wants to merge 2 commits intomainfrom
fix/relative-import&add-atom

Conversation

@rTreutlein
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Collaborator

@patham9 patham9 left a comment

Choose a reason for hiding this comment

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

I do not yet grasp this PR. What does it add exactly? Needs a test I guess to show? :)

@rTreutlein rTreutlein force-pushed the fix/relative-import&add-atom branch from 88bf9b3 to 0ba5536 Compare March 25, 2026 10:11
@rTreutlein
Copy link
Copy Markdown
Collaborator Author

There is a test for the relative imports and and i added one for the add/remove-atom change

Copy link
Copy Markdown
Collaborator

@patham9 patham9 left a comment

Choose a reason for hiding this comment

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

Translating expression on every add-atom / remove-atom call? Why is this necessary? This has significant performance impact, no?

@rTreutlein
Copy link
Copy Markdown
Collaborator Author

rTreutlein commented Apr 8, 2026

Translating the expression for the kb is not expensive in most cases it's just a symbol but now we support functions as well.
Also it seems the notifications are getting lost. So sorry for taking so long to answer.

@patham9
Copy link
Copy Markdown
Collaborator

patham9 commented Apr 15, 2026

Hm translating it again had significant overhead when I tried that on millions of items last time, maybe better to avoid it if possible? What does it have to do with relative imports anyways?

@rTreutlein
Copy link
Copy Markdown
Collaborator Author

rTreutlein commented Apr 15, 2026

@patham9 What do you mean translating it again before this change whatever was in the argument position for the space was just passe as the raw symbold so you could not do:

(= (kb) myspace)
!(add-atom (kb) (Atom To Add)

And it doesn't really have anything to do with the imports to me it was just such a small clear improvment that i didn't bother with another PR.

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.

2 participants